* [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
@ 2022-08-16 20:18 Name
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Name @ 2022-08-16 20:18 UTC (permalink / raw)
To: devel, Sami.Mujawar, Alexei.Fedorov, michael.d.kinney, gaoliming,
zhiguang.liu
Cc: Swatisri Kantamsetti
From: Swatisri Kantamsetti <swatisrik@nvidia.com>
Added MPAM table header, MSC and Resource Node
info structures
Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
---
MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
MdePkg/Include/IndustryStandard/Mpam.h | 69 ++++++++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
index fe5ebfac2b..e54f631186 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2952,6 +2952,11 @@ typedef struct {
///
#define EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('P', 'P', 'T', 'T')
+///
+/// "MPAM" Memory System Resource Partitioning And Monitoring Table
+///
+#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
+
///
/// "PSDT" Persistent System Description Table
///
diff --git a/MdePkg/Include/IndustryStandard/Mpam.h b/MdePkg/Include/IndustryStandard/Mpam.h
new file mode 100644
index 0000000000..e0f75f0114
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Mpam.h
@@ -0,0 +1,69 @@
+/** @file
+ ACPI Memory System Resource Partitioning And Monitoring (MPAM)
+ as specified in ARM spec DEN0065
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef _MPAM_H_
+#define _MPAM_H_
+
+#pragma pack(1)
+
+///
+/// Memory System Resource Partitioning and Monitoring Table (MPAM)
+///
+typedef struct {
+ EFI_ACPI_DESCRIPTION_HEADER Header;
+ UINT32 NumNodes;
+ UINT32 NodeOffset;
+ UINT32 Reserved;
+} EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER;
+
+///
+/// MPAM Revision (as defined in ACPI 6.4 spec.)
+///
+#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION 0x01
+
+///
+/// Memory System Controller Node Structure
+///
+
+typedef struct {
+ UINT16 Length;
+ UINT16 Reserved;
+ UINT32 Identifier;
+ UINT64 BaseAddress;
+ UINT32 MmioSize;
+ UINT32 OverflowInterrupt;
+ UINT32 OverflowInterruptFlags;
+ UINT32 Reserved1;
+ UINT32 OverflowInterruptAff;
+ UINT32 ErrorInterrupt;
+ UINT32 ErrorInterruptFlags;
+ UINT32 Reserved2;
+ UINT32 ErrorInterruptAff;
+ UINT32 MaxNRdyUsec;
+ UINT64 LinkedDeviceHwId;
+ UINT32 LinkedDeviceInstanceHwId;
+ UINT32 NumResourceNodes;
+} EFI_ACPI_6_4_MPAM_MSC_NODE;
+
+///
+/// Resource Node Structure
+///
+
+typedef struct {
+ UINT32 Identifier;
+ UINT8 RisIndex;
+ UINT16 Reserved1;
+ UINT8 LocatorType;
+ UINT64 Locator;
+ UINT32 NumFuncDep;
+} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
+
+#pragma pack()
+
+#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files
2022-08-16 20:18 [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Name
@ 2022-08-16 20:18 ` Name
2022-08-19 8:26 ` [edk2-devel] " Rohit Mathew
2022-08-23 17:51 ` Swatisri Kantamsetti
2022-08-19 8:26 ` [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Rohit Mathew
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Name @ 2022-08-16 20:18 UTC (permalink / raw)
To: devel, Sami.Mujawar, Alexei.Fedorov, michael.d.kinney, gaoliming,
zhiguang.liu
Cc: Swatisri Kantamsetti
From: Swatisri Kantamsetti <swatisrik@nvidia.com>
ACPI header, MSC and Resource Nodes are populated
in the MPAM Table
Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
---
DynamicTablesPkg/DynamicTables.dsc.inc | 2 +
DynamicTablesPkg/Include/AcpiTableGenerator.h | 1 +
.../Include/ArmNameSpaceObjects.h | 68 ++
.../Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf | 30 +
.../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c | 649 ++++++++++++++++++
.../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h | 47 ++
6 files changed, 797 insertions(+)
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 3d4fa0c4c4..745d5f0633 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -29,6 +29,7 @@
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
+ DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
@@ -54,6 +55,7 @@
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index f962dbff57..56d7375b4a 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -94,6 +94,7 @@ typedef enum StdAcpiTableId {
EStdAcpiTableIdIort, ///< IORT Generator
EStdAcpiTableIdPptt, ///< PPTT Generator
EStdAcpiTableIdSrat, ///< SRAT Generator
+ EStdAcpiTableIdMpam, ///< MPAM Generator
EStdAcpiTableIdSsdtSerialPort, ///< SSDT Serial-Port Generator
EStdAcpiTableIdSsdtCmn600, ///< SSDT Cmn-600 Generator
EStdAcpiTableIdSsdtCpuTopology, ///< SSDT Cpu Topology
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 102e0f96be..39a14ed0b3 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -63,6 +63,8 @@ typedef enum ArmObjectID {
EArmObjPciInterruptMapInfo, ///< 39 - Pci Interrupt Map Info
EArmObjRmr, ///< 40 - Reserved Memory Range Node
EArmObjMemoryRangeDescriptor, ///< 41 - Memory Range Descriptor
+ EArmObjMscNodeInfo, ///< 40 - Msc Memory System Controller Node Info
+ EArmObjResNodeInfo, ///< 41 - Res Resource Node Info
EArmObjMax
} EARM_OBJECT_ID;
@@ -1070,6 +1072,72 @@ typedef struct CmArmRmrDescriptor {
UINT64 Length;
} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
+/** A structure that describes Memory System Controller Node.
+
+ MPAM Memory System Component Nodes are described by
+ this object.
+
+ ID: EArmObjMscNodeInfo
+*/
+typedef struct CmArmMscNodeInfo {
+ /// An unique token used to identify this object
+ CM_OBJECT_TOKEN Token;
+
+ /// MPAM Base Address
+ UINT64 BaseAddress;
+ /// MMIO Size
+ UINT32 MmioSize;
+ /// Overflow Interrupt
+ UINT32 OverflowInterrupt;
+ /// Overflow Interrupt Flags
+ UINT32 OverflowInterruptFlags;
+ /// Overflow Interrupt Affinity
+ UINT32 OverflowInterruptAff;
+ /// Error Interrupt
+ UINT32 ErrorInterrupt;
+ /// Error Interrupt Flags
+ UINT32 ErrorInterruptFlags;
+ /// Error Interrupt Affinity
+ UINT32 ErrorInterruptAff;
+ /// Not Ready Signal time
+ UINT32 MaxNRdyUsec;
+ /// Linked Device HWID
+ UINT64 LinkedDeviceHwId;
+ /// Linked Device Instance ID
+ UINT32 LinkedDeviceInstanceHwId;
+ /// Number of Resource nodes
+ UINT32 NumResourceNodes;
+ /// Reference token for the list of resource nodes
+ //CM_OBJECT_TOKEN ResourceNodeListToken;
+
+} CM_ARM_MSC_NODE_INFO;
+
+/** A structure that describes Memory System Controller Node.
+
+ MPAM Memory System Component Nodes are described by
+ this object.
+
+ ID: EArmObjResNodeInfo
+*/
+typedef struct CmArmResNodeInfo {
+ /// An unique token used to identify this object
+ CM_OBJECT_TOKEN Token;
+
+ /// Identifier
+ UINT32 Identifier;
+ /// RIS Index
+ UINT8 RisIndex;
+ /// Locator Type
+ UINT8 LocatorType;
+ /// Locator
+ UINT64 Locator;
+ /// Num functional dependencies
+ UINT32 NumFuncDep;
+ /// Reference token for the list of resource nodes
+ CM_OBJECT_TOKEN FuncDepListToken;
+
+} CM_ARM_RESOURCE_NODE_INFO;
+
#pragma pack()
#endif // ARM_NAMESPACE_OBJECTS_H_
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
new file mode 100644
index 0000000000..480130dc21
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
@@ -0,0 +1,30 @@
+## @file
+# MPAM Table Generator Inf file
+#
+# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+# Copyright (c) 2022, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = AcpiMpamLibArm
+ FILE_GUID = 02d0c79f-41cd-45c9-9835-781229c619d1
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = NULL|DXE_DRIVER
+ CONSTRUCTOR = AcpiMpamLibConstructor
+ DESTRUCTOR = AcpiMpamLibDestructor
+
+[Sources]
+ MpamGenerator.c
+ MpamGenerator.h
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ DynamicTablesPkg/DynamicTablesPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
new file mode 100644
index 0000000000..db3e8e95bc
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
@@ -0,0 +1,649 @@
+/** @file
+ MPAM Table Generator
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.4 Specification, January 2021
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#include <IndustryStandard/Mpam.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>
+
+#include "MpamGenerator.h"
+
+/**
+ ARM standard MPAM Generator
+
+ Requirements:
+ The following Configuration Manager Object(s) are used by this Generator:
+ - EArmObjMscNodeInfo (REQUIRED)
+ - EArmObjResNodeInfo
+*/
+
+/**
+ This macro expands to a function that retrieves the MSC Node information
+ from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceArm,
+ EArmObjMscNodeInfo,
+ CM_ARM_MSC_NODE_INFO
+ );
+
+/**
+ This macro expands to a function that retrieves the Resource Node
+ information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceArm,
+ EArmObjResNodeInfo,
+ CM_ARM_RESOURCE_NODE_INFO
+ );
+
+/**
+ Returns the size of the MPAM Memory System Controller (MSC) Node
+
+ @param [in] Node Pointer to MSC Node Info CM object
+
+ @retval Size of the MSC Node in bytes.
+**/
+STATIC
+UINT32
+GetMscNodeSize (
+ IN CONST CM_ARM_MSC_NODE_INFO *Node
+ )
+{
+ ASSERT (Node != NULL);
+
+ // <size of Memory System Controller Node>
+ return (UINT32)(sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE) +
+ Node->NumResourceNodes * sizeof (EFI_ACPI_6_4_MPAM_RESOURCE_NODE));
+}
+
+/** Returns the total size required for the MSC and
+ updates the Node Indexer.
+
+ This function calculates the size required for the node group
+ and also populates the Node Indexer array with offsets for the
+ individual nodes.
+
+ @param [in] NodeStartOffset Offset from the start of the
+ MPAM where this node group starts.
+ @param [in] NodeList Pointer to MSC Group node list.
+ @param [in] NodeCount Count of the MSC Group nodes.
+ @param [in, out] NodeIndexer Pointer to the next Node Indexer.
+
+ @retval Total size of the ITS Group Nodes.
+**/
+STATIC
+UINT64
+GetSizeofMscGroupNodes (
+ IN CONST UINT32 NodeStartOffset,
+ IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
+ IN UINT32 NodeCount,
+ IN OUT MPAM_NODE_INDEXER **CONST NodeIndexer
+ )
+{
+ UINT64 Size;
+
+ ASSERT (NodeList != NULL);
+
+ Size = 0;
+ while (NodeCount-- != 0) {
+ (*NodeIndexer)->Token = NodeList->Token;
+ (*NodeIndexer)->Object = (VOID *)NodeList;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+ DEBUG ((
+ DEBUG_INFO,
+ "MPAM: MSC Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+ *NodeIndexer,
+ (*NodeIndexer)->Token,
+ (*NodeIndexer)->Object,
+ (*NodeIndexer)->Offset
+ ));
+
+ Size += GetMscNodeSize (NodeList);
+
+ (*NodeIndexer)++;
+ NodeList++;
+ }
+
+ return Size;
+}
+
+/** Update the MSC Group Node Information.
+
+ @param [in] This Pointer to the table Generator.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] Mpam Pointer to MPAM table structure.
+ @param [in] NodesStartOffset Offset for the start of the Msc Group
+ Nodes.
+ @param [in] NodeList Pointer to an array of Msc Group Node
+ Objects.
+ @param [in] NodeCount Number of Msc Group Node Objects.
+
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object was not found.
+**/
+STATIC
+EFI_STATUS
+AddMscNodes (
+ IN CONST ACPI_TABLE_GENERATOR *CONST This,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *Mpam,
+ IN CONST UINT32 NodesStartOffset,
+ IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
+ IN UINT32 NodeCount
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_6_4_MPAM_MSC_NODE *MscNode;
+ EFI_ACPI_6_4_MPAM_RESOURCE_NODE *ResourceNodeArray;
+ CM_ARM_RESOURCE_NODE_INFO *ResourceNodeList;
+ UINT64 NodeLength;
+ UINT32 ResourceNodeCount;
+
+ ASSERT (Mpam != NULL);
+
+ MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)Mpam + NodesStartOffset);
+
+ // Get the Resource Node info to update the MPAM MSC node
+ Status = GetEArmObjResNodeInfo (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &ResourceNodeList,
+ &ResourceNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add resource nodes info. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ while (NodeCount-- != 0) {
+ NodeLength = GetMscNodeSize (NodeList);
+
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: MSC Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
+ // Populate the node header
+ MscNode->Length = (UINT16)NodeLength;
+ MscNode->Reserved = EFI_ACPI_RESERVED_WORD;
+ MscNode->Identifier = EFI_ACPI_RESERVED_DWORD;
+ MscNode->BaseAddress = NodeList->BaseAddress;
+ MscNode->MmioSize = NodeList->MmioSize;
+ MscNode->OverflowInterrupt = NodeList->OverflowInterrupt;
+ MscNode->OverflowInterruptFlags = NodeList->OverflowInterruptFlags;
+ MscNode->Reserved1 = EFI_ACPI_RESERVED_DWORD;
+ MscNode->OverflowInterruptAff = NodeList->OverflowInterruptAff;
+ MscNode->ErrorInterrupt = NodeList->ErrorInterrupt;
+ MscNode->ErrorInterruptFlags = NodeList->ErrorInterruptFlags;
+ MscNode->Reserved2 = EFI_ACPI_RESERVED_DWORD;
+ MscNode->ErrorInterruptAff = NodeList->ErrorInterruptAff;
+ MscNode->MaxNRdyUsec = NodeList->MaxNRdyUsec;
+ MscNode->LinkedDeviceHwId = NodeList->LinkedDeviceHwId;
+ MscNode->LinkedDeviceInstanceHwId = NodeList->LinkedDeviceInstanceHwId;
+ MscNode->NumResourceNodes = NodeList->NumResourceNodes;
+
+ // ResourceNode List for each MSC
+ if (MscNode->NumResourceNodes > 0) {
+ // Resource Node array for this Msc node
+ ResourceNodeArray = (EFI_ACPI_6_4_MPAM_RESOURCE_NODE *)((UINT8 *)MscNode + sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE));
+ // Adding Resource Node content
+ while ( MscNode->NumResourceNodes-- != 0 ) {
+ ResourceNodeArray->Identifier = ResourceNodeList->Identifier;
+ ResourceNodeArray->RisIndex = ResourceNodeList->RisIndex;
+ ResourceNodeArray->Reserved1 = EFI_ACPI_RESERVED_WORD;
+ ResourceNodeArray->LocatorType = ResourceNodeList->LocatorType;
+ ResourceNodeArray->Locator = ResourceNodeList->Locator;
+ ResourceNodeArray->NumFuncDep = ResourceNodeList->NumFuncDep;
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add Resource Node List. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ ResourceNodeList++;
+ ResourceNodeArray++;
+ }
+ }
+
+ // Next MSC Node
+ MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)MscNode + MscNode->Length);
+ NodeList++;
+ } // Msc Node
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Construct the MPAM ACPI table.
+
+ 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 generator to be used.
+ @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.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildMpamTable (
+ 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;
+ UINT64 TableSize;
+ UINT64 NodeSize;
+ UINT32 MpamNodeCount;
+
+ UINT32 MscNodeCount;
+ UINT32 MscNodeOffset;
+
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *Mpam;
+ ACPI_MPAM_GENERATOR *Generator;
+ CM_ARM_MSC_NODE_INFO *MscNodeList;
+ MPAM_NODE_INDEXER *NodeIndexer;
+
+ ASSERT (
+ (This != NULL) &&
+ (AcpiTableInfo != NULL) &&
+ (CfgMgrProtocol != NULL) &&
+ (Table != NULL) &&
+ (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
+ (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
+ );
+
+ DEBUG ((
+ DEBUG_ERROR,
+ "DEBUG PRINT: MPAM: Requested table revision = %d\n",
+ AcpiTableInfo->AcpiTableRevision
+ ));
+
+ if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
+ (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision))
+ {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: 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;
+ }
+
+ Generator = (ACPI_MPAM_GENERATOR *)This;
+ *Table = NULL;
+
+ // Get the Memory System Controller Node info and update the MPAM
+ // structure count with MSC Node count (Type 0)
+ Status = GetEArmObjMscNodeInfo (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &MscNodeList,
+ &MscNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to get memory system controller node info. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ MpamNodeCount = MscNodeCount;
+ Generator->MscNodeCount = MscNodeCount;
+
+ // Allocate Node Indexer array
+ NodeIndexer = (MPAM_NODE_INDEXER *)AllocateZeroPool (
+ sizeof (MPAM_NODE_INDEXER) *
+ MpamNodeCount
+ );
+ if (NodeIndexer == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to allocate memory for Node Indexer. Status = %r\n ",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ DEBUG ((DEBUG_INFO, "MPAM INFO: NodeIndexer = %p\n", NodeIndexer));
+ Generator->MpamNodeCount = MpamNodeCount;
+ Generator->NodeIndexer = NodeIndexer;
+
+ // Calculate the size of the MPAM table
+ TableSize = sizeof (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER);
+
+ // Include the size of MSC Nodes and index them
+ if (Generator->MscNodeCount != 0) {
+ MscNodeOffset = TableSize;
+ // Size of MSC nodes.
+ NodeSize = GetSizeofMscGroupNodes (
+ MscNodeOffset,
+ MscNodeList,
+ Generator->MscNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Invalid Size of Group Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " MscNodeCount = %d\n" \
+ " MscNodeOffset = 0x%x\n",
+ Generator->MscNodeCount,
+ MscNodeOffset
+ ));
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: MPAM:\n" \
+ " MpamNodeCount = %d\n" \
+ " TableSize = 0x%X\n",
+ MpamNodeCount,
+ TableSize
+ ));
+
+ if (TableSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: MPAM Table Size 0x%lx > MAX_UINT32," \
+ " Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
+ // Allocate the Buffer for the MPAM table
+ *Table = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize);
+ if (*Table == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to allocate memory for MPAM Table. " \
+ "Size = %d. Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
+ Mpam = (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *)*Table;
+
+ DEBUG ((
+ DEBUG_INFO,
+ "MPAM: Mpam = 0x%p. TableSize = 0x%x\n",
+ Mpam,
+ TableSize
+ ));
+
+ // Add ACPI header
+ Status = AddAcpiHeader (
+ CfgMgrProtocol,
+ This,
+ &Mpam->Header,
+ AcpiTableInfo,
+ TableSize
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add ACPI header. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ // Update MPAM table
+ Mpam->NumNodes = MscNodeCount;
+ Mpam->NodeOffset = sizeof (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER);
+ Mpam->Reserved = EFI_ACPI_RESERVED_DWORD;
+
+ // Add MSC Nodes to the generated table
+ if (Mpam->NumNodes != 0) {
+ Status = AddMscNodes (
+ This,
+ CfgMgrProtocol,
+ Mpam,
+ MscNodeOffset,
+ MscNodeList,
+ MscNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add MSC Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ }
+
+ return Status;
+
+error_handler:
+ if (Generator->NodeIndexer != NULL) {
+ FreePool (Generator->NodeIndexer);
+ Generator->NodeIndexer = NULL;
+ }
+
+ if (*Table != NULL) {
+ FreePool (*Table);
+ *Table = NULL;
+ }
+
+ return Status;
+}
+
+/** Free any resources allocated for constructing the MPAM
+
+ @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
+FreeMpamTableResources (
+ 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
+ )
+{
+ ACPI_MPAM_GENERATOR *Generator;
+
+ ASSERT (This != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
+ ASSERT (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature);
+
+ Generator = (ACPI_MPAM_GENERATOR *)This;
+
+ // Free any memory allocated by the generator
+ if (Generator->NodeIndexer != NULL) {
+ FreePool (Generator->NodeIndexer);
+ Generator->NodeIndexer = NULL;
+ }
+
+ if ((Table == NULL) || (*Table == NULL)) {
+ DEBUG ((DEBUG_ERROR, "ERROR: MPAM: Invalid Table Pointer\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ FreePool (*Table);
+ *Table = NULL;
+
+ return EFI_SUCCESS;
+}
+
+/** The MPAM Table Generator revision.
+*/
+#define MPAM_GENERATOR_REVISION CREATE_REVISION (1, 0)
+
+/** The interface for the MPAM Table Generator.
+*/
+STATIC
+ACPI_MPAM_GENERATOR MpamGenerator = {
+ // ACPI table generator header
+ {
+ // Generator ID
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMpam),
+ // Generator Description
+ L"ACPI.STD.MPAM.GENERATOR",
+ // ACPI Table Signature
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_STRUCTURE_SIGNATURE,
+ // ACPI Table Revision supported by this Generator
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION,
+ // Minimum supported ACPI Table Revision
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION,
+ // Creator ID
+ TABLE_GENERATOR_CREATOR_ID_ARM,
+ // Creator Revision
+ MPAM_GENERATOR_REVISION,
+ // Build Table function
+ BuildMpamTable,
+ // Free Resource function
+ FreeMpamTableResources,
+ // Extended build function not needed
+ NULL,
+ // Extended build function not implemented by the generator.
+ // Hence extended free resource function is not required.
+ NULL
+ },
+
+ // MPAM Generator private data
+
+ // MPAM node count
+ 0,
+ // MSC node count
+ 0,
+
+ // Pointer to MPAM Node Indexer
+ 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
+AcpiMpamLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = RegisterAcpiTableGenerator (&MpamGenerator.Header);
+ DEBUG ((DEBUG_INFO, "MPAM: 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
+AcpiMpamLibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = DeregisterAcpiTableGenerator (&MpamGenerator.Header);
+ DEBUG ((DEBUG_INFO, "MPAM: Deregister Generator. Status = %r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
new file mode 100644
index 0000000000..1075bd8c6c
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
@@ -0,0 +1,47 @@
+/** @file
+ Header file for the dynamic MPAM generator
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.4 Specification, January 2021
+ - ARM Architecture Reference Manual ARMv8
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#ifndef MPAM_GENERATOR_H_
+#define MPAM_GENERATOR_H_
+
+#pragma pack(1)
+
+/** A structure that describes the Node indexer
+ used for indexing the MPAM MSC nodes.
+*/
+typedef struct MpamNodeIndexer {
+ /// Index token for the Node
+ CM_OBJECT_TOKEN Token;
+ /// Pointer to the node
+ VOID *Object;
+ /// Node offset from the start of the MPAM table
+ UINT32 Offset;
+} MPAM_NODE_INDEXER;
+
+typedef struct AcpiMpamGenerator {
+ /// ACPI Table generator header
+ ACPI_TABLE_GENERATOR Header;
+ /// MPAM structure count
+ UINT32 MpamNodeCount;
+ /// Count of Msc Nodes
+ UINT32 MscNodeCount;
+ /// List of indexed CM objects for MPAM generation
+ MPAM_NODE_INDEXER *NodeIndexer;
+} ACPI_MPAM_GENERATOR;
+
+#pragma pack()
+
+#endif // MPAM_GENERATOR_H_
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-16 20:18 [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Name
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
@ 2022-08-19 8:26 ` Rohit Mathew
2022-08-23 20:10 ` Andrew Fish
2022-08-23 17:52 ` Swatisri Kantamsetti
2022-10-03 11:17 ` Sami Mujawar
3 siblings, 1 reply; 15+ messages in thread
From: Rohit Mathew @ 2022-08-19 8:26 UTC (permalink / raw)
To: devel@edk2.groups.io, username@nvidia.com, Sami Mujawar,
Alexei Fedorov, michael.d.kinney@intel.com,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com
Cc: Swatisri Kantamsetti, Thomas Abraham, Thanu Rangarajan, nd
Hi Swatisri,
Thanks for the patch. Please find my comments inline marked [Rohit] -
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Name
> via groups.io
> Sent: 16 August 2022 21:18
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>;
> Alexei Fedorov <Alexei.Fedorov@arm.com>; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com
> Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
> Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
>
> From: Swatisri Kantamsetti <swatisrik@nvidia.com>
>
> Added MPAM table header, MSC and Resource Node info structures
>
> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> ---
> MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
> MdePkg/Include/IndustryStandard/Mpam.h | 69
> ++++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
> create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
>
> diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
> b/MdePkg/Include/IndustryStandard/Acpi64.h
> index fe5ebfac2b..e54f631186 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi64.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi64.h
> @@ -2952,6 +2952,11 @@ typedef struct {
> ///
> #define
> EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SI
> GNATURE SIGNATURE_32('P', 'P', 'T', 'T')
>
> +///
> +/// "MPAM" Memory System Resource Partitioning And Monitoring Table
> ///
> +#define
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_STRUC
> +TURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
> +
> ///
> /// "PSDT" Persistent System Description Table /// diff --git
> a/MdePkg/Include/IndustryStandard/Mpam.h
> b/MdePkg/Include/IndustryStandard/Mpam.h
> new file mode 100644
> index 0000000000..e0f75f0114
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Mpam.h
> @@ -0,0 +1,69 @@
> +/** @file
> + ACPI Memory System Resource Partitioning And Monitoring (MPAM)
> + as specified in ARM spec DEN0065
> +
> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2022, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#ifndef _MPAM_H_
> +#define _MPAM_H_
> +
> +#pragma pack(1)
> +
> +///
> +/// Memory System Resource Partitioning and Monitoring Table (MPAM)
> ///
> +typedef struct {
> + EFI_ACPI_DESCRIPTION_HEADER Header;
> + UINT32 NumNodes;
> + UINT32 NodeOffset;
> + UINT32 Reserved;
> +}
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_HEADE
> +R;
[Rohit] Shouldn't the header be followed by MSC node body type as defined in MPAM ACPI 1.0, section 2, table 3 - The MPAM table and section 2.1, table 4 - MSC Node body?
> +
> +///
> +/// MPAM Revision (as defined in ACPI 6.4 spec.) /// #define
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_REVIS
> +ION 0x01
> +
> +///
> +/// Memory System Controller Node Structure ///
> +
> +typedef struct {
> + UINT16 Length;
> + UINT16 Reserved;
> + UINT32 Identifier;
> + UINT64 BaseAddress;
> + UINT32 MmioSize;
> + UINT32 OverflowInterrupt;
> + UINT32 OverflowInterruptFlags;
[Rohit] Would it be better to have a type (possibly bitfield struct) instead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - Interrupt flags)
> + UINT32 Reserved1;
> + UINT32 OverflowInterruptAff;
> + UINT32 ErrorInterrupt;
> + UINT32 ErrorInterruptFlags;
[Rohit ] Same comment as before above.
> + UINT32 Reserved2;
> + UINT32 ErrorInterruptAff;
> + UINT32 MaxNRdyUsec;
> + UINT64 LinkedDeviceHwId;
> + UINT32 LinkedDeviceInstanceHwId;
> + UINT32 NumResourceNodes;
> +} EFI_ACPI_6_4_MPAM_MSC_NODE;
> +
> +///
> +/// Resource Node Structure
> +///
> +
> +typedef struct {
> + UINT32 Identifier;
> + UINT8 RisIndex;
> + UINT16 Reserved1;
> + UINT8 LocatorType;
> + UINT64 Locator;
[Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
> + UINT32 NumFuncDep;
> +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
[Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NODE type and this could be non-zero, should we also need the type for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Functional dependency descriptor)
[Rohit] Also, could some of the commonly used macros be added to this header, please? (location types, MPAM interrupt mode, interrupt types, affinity type, etc)
> +
> +#pragma pack()
> +
> +#endif
> --
> 2.17.1
>
>
>
>
>
Regards,
Rohit
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
@ 2022-08-19 8:26 ` Rohit Mathew
2022-08-24 10:51 ` Rohit Mathew
2022-08-23 17:51 ` Swatisri Kantamsetti
1 sibling, 1 reply; 15+ messages in thread
From: Rohit Mathew @ 2022-08-19 8:26 UTC (permalink / raw)
To: devel@edk2.groups.io, username@nvidia.com, Sami Mujawar,
Alexei Fedorov, michael.d.kinney@intel.com,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com
Cc: Swatisri Kantamsetti, Thomas Abraham, Thanu Rangarajan, nd
Hi Swatisri,
Thanks for the patch. Please find my comments inline marked [Rohit] -
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Name
> via groups.io
> Sent: 16 August 2022 21:18
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>;
> Alexei Fedorov <Alexei.Fedorov@arm.com>; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com
> Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
> Subject: [edk2-devel] [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM
> Generator and supporting files
>
> From: Swatisri Kantamsetti <swatisrik@nvidia.com>
>
> ACPI header, MSC and Resource Nodes are populated in the MPAM Table
>
> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> ---
> DynamicTablesPkg/DynamicTables.dsc.inc | 2 +
> DynamicTablesPkg/Include/AcpiTableGenerator.h | 1 +
> .../Include/ArmNameSpaceObjects.h | 68 ++
> .../Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf | 30 +
> .../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c | 649
> ++++++++++++++++++
> .../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h | 47 ++
> 6 files changed, 797 insertions(+)
> create mode 100644
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.in
> f
> create mode 100644
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
> create mode 100644
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
>
> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc
> b/DynamicTablesPkg/DynamicTables.dsc.inc
> index 3d4fa0c4c4..745d5f0633 100644
> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> @@ -29,6 +29,7 @@
> DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
> +
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.in
> f
> DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
> DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
> @@ -54,6 +55,7 @@
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibAr
> m.inf
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibAr
> m.inf
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.
> inf
> +
> +
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLib
> Arm.i
> + nf
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.
> inf
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.
> inf
>
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.i
> nf
> diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h
> b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> index f962dbff57..56d7375b4a 100644
> --- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
> +++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> @@ -94,6 +94,7 @@ typedef enum StdAcpiTableId {
> EStdAcpiTableIdIort, ///< IORT Generator
> EStdAcpiTableIdPptt, ///< PPTT Generator
> EStdAcpiTableIdSrat, ///< SRAT Generator
> + EStdAcpiTableIdMpam, ///< MPAM Generator
> EStdAcpiTableIdSsdtSerialPort, ///< SSDT Serial-Port Generator
> EStdAcpiTableIdSsdtCmn600, ///< SSDT Cmn-600 Generator
> EStdAcpiTableIdSsdtCpuTopology, ///< SSDT Cpu Topology
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 102e0f96be..39a14ed0b3 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -63,6 +63,8 @@ typedef enum ArmObjectID {
> EArmObjPciInterruptMapInfo, ///< 39 - Pci Interrupt Map Info
> EArmObjRmr, ///< 40 - Reserved Memory Range Node
> EArmObjMemoryRangeDescriptor, ///< 41 - Memory Range Descriptor
> + EArmObjMscNodeInfo, ///< 40 - Msc Memory System Controller
> Node Info
> + EArmObjResNodeInfo, ///< 41 - Res Resource Node Info
> EArmObjMax
> } EARM_OBJECT_ID;
>
> @@ -1070,6 +1072,72 @@ typedef struct CmArmRmrDescriptor {
> UINT64 Length;
> } CM_ARM_MEMORY_RANGE_DESCRIPTOR;
>
> +/** A structure that describes Memory System Controller Node.
> +
> + MPAM Memory System Component Nodes are described by
> + this object.
> +
> + ID: EArmObjMscNodeInfo
> +*/
> +typedef struct CmArmMscNodeInfo {
> + /// An unique token used to identify this object
> + CM_OBJECT_TOKEN Token;
> +
> + /// MPAM Base Address
> + UINT64 BaseAddress;
> + /// MMIO Size
> + UINT32 MmioSize;
> + /// Overflow Interrupt
> + UINT32 OverflowInterrupt;
> + /// Overflow Interrupt Flags
> + UINT32 OverflowInterruptFlags;
[Rohit] I have added a comment on Flag's type in [PATCH 1/2]. Same comment here.
> + /// Overflow Interrupt Affinity
> + UINT32 OverflowInterruptAff;
> + /// Error Interrupt
> + UINT32 ErrorInterrupt;
> + /// Error Interrupt Flags
> + UINT32 ErrorInterruptFlags;
> + /// Error Interrupt Affinity
> + UINT32 ErrorInterruptAff;
> + /// Not Ready Signal time
> + UINT32 MaxNRdyUsec;
> + /// Linked Device HWID
> + UINT64 LinkedDeviceHwId;
> + /// Linked Device Instance ID
> + UINT32 LinkedDeviceInstanceHwId;
> + /// Number of Resource nodes
> + UINT32 NumResourceNodes;
> + /// Reference token for the list of resource nodes
> + //CM_OBJECT_TOKEN ResourceNodeListToken;
> +
> +} CM_ARM_MSC_NODE_INFO;
> +
> +/** A structure that describes Memory System Controller Node.
> +
> + MPAM Memory System Component Nodes are described by
> + this object.
> +
> + ID: EArmObjResNodeInfo
> +*/
> +typedef struct CmArmResNodeInfo {
> + /// An unique token used to identify this object
> + CM_OBJECT_TOKEN Token;
> +
> + /// Identifier
> + UINT32 Identifier;
> + /// RIS Index
> + UINT8 RisIndex;
> + /// Locator Type
> + UINT8 LocatorType;
> + /// Locator
> + UINT64 Locator;
[Rohit] I have added a comment on Locator's type and size in [PATCH 1/2]. Same comment here.
> + /// Num functional dependencies
> + UINT32 NumFuncDep;
> + /// Reference token for the list of resource nodes
> + CM_OBJECT_TOKEN FuncDepListToken;
> +
> +} CM_ARM_RESOURCE_NODE_INFO;
> +
> #pragma pack()
>
> #endif // ARM_NAMESPACE_OBJECTS_H_
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> .inf
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> .inf
> new file mode 100644
> index 0000000000..480130dc21
> --- /dev/null
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> .in
> +++ f
> @@ -0,0 +1,30 @@
> +## @file
> +# MPAM Table Generator Inf file
> +#
> +# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> +# Copyright (c) 2022, ARM Limited. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> + INF_VERSION = 0x0001001B
> + BASE_NAME = AcpiMpamLibArm
> + FILE_GUID = 02d0c79f-41cd-45c9-9835-781229c619d1
> + VERSION_STRING = 1.0
> + MODULE_TYPE = DXE_DRIVER
> + LIBRARY_CLASS = NULL|DXE_DRIVER
> + CONSTRUCTOR = AcpiMpamLibConstructor
> + DESTRUCTOR = AcpiMpamLibDestructor
> +
> +[Sources]
> + MpamGenerator.c
> + MpamGenerator.h
> +
> +[Packages]
> + EmbeddedPkg/EmbeddedPkg.dec
> + DynamicTablesPkg/DynamicTablesPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> c
> new file mode 100644
> index 0000000000..db3e8e95bc
> --- /dev/null
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> c
> @@ -0,0 +1,649 @@
> +/** @file
> + MPAM Table Generator
> +
> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2022, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Reference(s):
> + - ACPI 6.4 Specification, January 2021
> +
> + @par Glossary:
> + - Cm or CM - Configuration Manager
> + - Obj or OBJ - Object
> +**/
> +
> +#include <IndustryStandard/Mpam.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>
> +
> +#include "MpamGenerator.h"
> +
> +/**
> + ARM standard MPAM Generator
> +
> + Requirements:
> + The following Configuration Manager Object(s) are used by this
> Generator:
> + - EArmObjMscNodeInfo (REQUIRED)
> + - EArmObjResNodeInfo
> +*/
> +
> +/**
> + This macro expands to a function that retrieves the MSC Node
> +information
> + from the Configuration Manager.
> +*/
> +GET_OBJECT_LIST (
> + EObjNameSpaceArm,
> + EArmObjMscNodeInfo,
> + CM_ARM_MSC_NODE_INFO
> + );
> +
> +/**
> + This macro expands to a function that retrieves the Resource Node
> + information from the Configuration Manager.
> +*/
> +GET_OBJECT_LIST (
> + EObjNameSpaceArm,
> + EArmObjResNodeInfo,
> + CM_ARM_RESOURCE_NODE_INFO
> + );
> +
> +/**
> + Returns the size of the MPAM Memory System Controller (MSC) Node
> +
> + @param [in] Node Pointer to MSC Node Info CM object
> +
> + @retval Size of the MSC Node in bytes.
> +**/
> +STATIC
> +UINT32
> +GetMscNodeSize (
> + IN CONST CM_ARM_MSC_NODE_INFO *Node
> + )
> +{
> + ASSERT (Node != NULL);
> +
> + // <size of Memory System Controller Node>
> + return (UINT32)(sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE) +
> + Node->NumResourceNodes * sizeof
> +(EFI_ACPI_6_4_MPAM_RESOURCE_NODE));
[Rohit] Shouldn't the node size include the size of functional dependency descriptor * number of functional dependency descriptor?
> +}
> +
> +/** Returns the total size required for the MSC and
> + updates the Node Indexer.
> +
> + This function calculates the size required for the node group
> + and also populates the Node Indexer array with offsets for the
> + individual nodes.
> +
> + @param [in] NodeStartOffset Offset from the start of the
> + MPAM where this node group starts.
[Rohit] Should it be "start of the MPAM table where .."?
> + @param [in] NodeList Pointer to MSC Group node list.
> + @param [in] NodeCount Count of the MSC Group nodes.
> + @param [in, out] NodeIndexer Pointer to the next Node Indexer.
> +
> + @retval Total size of the ITS Group Nodes.
> +**/
> +STATIC
> +UINT64
> +GetSizeofMscGroupNodes (
> + IN CONST UINT32 NodeStartOffset,
> + IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
> + IN UINT32 NodeCount,
> + IN OUT MPAM_NODE_INDEXER **CONST NodeIndexer
> + )
> +{
> + UINT64 Size;
> +
> + ASSERT (NodeList != NULL);
> +
> + Size = 0;
> + while (NodeCount-- != 0) {
> + (*NodeIndexer)->Token = NodeList->Token;
> + (*NodeIndexer)->Object = (VOID *)NodeList;
> + (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
> + DEBUG ((
> + DEBUG_INFO,
> + "MPAM: MSC Node Indexer = %p, Token = %p, Object = %p, Offset =
> 0x%x\n",
> + *NodeIndexer,
> + (*NodeIndexer)->Token,
> + (*NodeIndexer)->Object,
> + (*NodeIndexer)->Offset
> + ));
> +
> + Size += GetMscNodeSize (NodeList);
> +
> + (*NodeIndexer)++;
> + NodeList++;
> + }
> +
> + return Size;
> +}
> +
> +/** Update the MSC Group Node Information.
> +
> + @param [in] This Pointer to the table Generator.
> + @param [in] CfgMgrProtocol Pointer to the Configuration Manager
> + Protocol Interface.
> + @param [in] Mpam Pointer to MPAM table structure.
> + @param [in] NodesStartOffset Offset for the start of the Msc Group
> + Nodes.
> + @param [in] NodeList Pointer to an array of Msc Group Node
> + Objects.
> + @param [in] NodeCount Number of Msc Group Node Objects.
> +
> + @retval EFI_SUCCESS Table generated successfully.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> + @retval EFI_NOT_FOUND The required object was not found.
> +**/
> +STATIC
> +EFI_STATUS
> +AddMscNodes (
> + IN CONST ACPI_TABLE_GENERATOR *CONST
> This,
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL
> *CONST CfgMgrProtocol,
> + IN CONST
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEADER *Mpam,
> + IN CONST UINT32
> NodesStartOffset,
> + IN CONST CM_ARM_MSC_NODE_INFO
> *NodeList,
> + IN UINT32 NodeCount
> + )
> +{
> + EFI_STATUS Status;
> + EFI_ACPI_6_4_MPAM_MSC_NODE *MscNode;
> + EFI_ACPI_6_4_MPAM_RESOURCE_NODE *ResourceNodeArray;
> + CM_ARM_RESOURCE_NODE_INFO *ResourceNodeList;
> + UINT64 NodeLength;
> + UINT32 ResourceNodeCount;
> +
> + ASSERT (Mpam != NULL);
> +
> + MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)Mpam +
> + NodesStartOffset);
> +
> + // Get the Resource Node info to update the MPAM MSC node Status =
> + GetEArmObjResNodeInfo (
> + CfgMgrProtocol,
> + CM_NULL_TOKEN,
> + &ResourceNodeList,
> + &ResourceNodeCount
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to add resource nodes info. Status = %r\n",
> + Status
> + ));
> + return Status;
> + }
> +
> + while (NodeCount-- != 0) {
> + NodeLength = GetMscNodeSize (NodeList);
> +
> + if (NodeLength > MAX_UINT16) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: MSC Node length 0x%lx > MAX_UINT16. Status =
> %r\n",
> + NodeLength,
> + Status
> + ));
> + return Status;
> + }
> +
> + // Populate the node header
> + MscNode->Length = (UINT16)NodeLength;
> + MscNode->Reserved = EFI_ACPI_RESERVED_WORD;
> + MscNode->Identifier = EFI_ACPI_RESERVED_DWORD;
[Rohit] MPAM ACPI 1.0, section 2.1, table 4 - MSC node body, defines identifier as a unique value. For systems with multiple MSCs, the OS could expect identifiers to be unique. Could this be addressed, please?
> + MscNode->BaseAddress = NodeList->BaseAddress;
> + MscNode->MmioSize = NodeList->MmioSize;
> + MscNode->OverflowInterrupt = NodeList->OverflowInterrupt;
> + MscNode->OverflowInterruptFlags = NodeList->OverflowInterruptFlags;
> + MscNode->Reserved1 = EFI_ACPI_RESERVED_DWORD;
> + MscNode->OverflowInterruptAff = NodeList->OverflowInterruptAff;
> + MscNode->ErrorInterrupt = NodeList->ErrorInterrupt;
> + MscNode->ErrorInterruptFlags = NodeList->ErrorInterruptFlags;
> + MscNode->Reserved2 = EFI_ACPI_RESERVED_DWORD;
> + MscNode->ErrorInterruptAff = NodeList->ErrorInterruptAff;
> + MscNode->MaxNRdyUsec = NodeList->MaxNRdyUsec;
> + MscNode->LinkedDeviceHwId = NodeList->LinkedDeviceHwId;
> + MscNode->LinkedDeviceInstanceHwId = NodeList-
> >LinkedDeviceInstanceHwId;
> + MscNode->NumResourceNodes = NodeList->NumResourceNodes;
> +
> + // ResourceNode List for each MSC
> + if (MscNode->NumResourceNodes > 0) {
> + // Resource Node array for this Msc node
> + ResourceNodeArray = (EFI_ACPI_6_4_MPAM_RESOURCE_NODE
> *)((UINT8 *)MscNode + sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE));
> + // Adding Resource Node content
> + while ( MscNode->NumResourceNodes-- != 0 ) {
> + ResourceNodeArray->Identifier = ResourceNodeList->Identifier;
> + ResourceNodeArray->RisIndex = ResourceNodeList->RisIndex;
> + ResourceNodeArray->Reserved1 = EFI_ACPI_RESERVED_WORD;
> + ResourceNodeArray->LocatorType = ResourceNodeList->LocatorType;
> + ResourceNodeArray->Locator = ResourceNodeList->Locator;
> + ResourceNodeArray->NumFuncDep = ResourceNodeList-
> >NumFuncDep;
[Rohit] If NumFuncDep > 0, shouldn't we need an additional loop to populate functional dependency list?
> +
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to add Resource Node List. Status = %r\n",
> + Status
> + ));
> + return Status;
> + }
> +
> + ResourceNodeList++;
> + ResourceNodeArray++;
> + }
> + }
> +
> + // Next MSC Node
> + MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)MscNode +
> MscNode->Length);
> + NodeList++;
> + } // Msc Node
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Construct the MPAM ACPI table.
> +
> + 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 generator to be
> used.
> + @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.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +BuildMpamTable (
> + 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;
> + UINT64 TableSize;
> + UINT64 NodeSize;
> + UINT32 MpamNodeCount;
> +
> + UINT32 MscNodeCount;
> + UINT32 MscNodeOffset;
> +
> +
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEADER *Mpam;
> + ACPI_MPAM_GENERATOR *Generator;
> + CM_ARM_MSC_NODE_INFO *MscNodeList;
> + MPAM_NODE_INDEXER *NodeIndexer;
> +
> + ASSERT (
> + (This != NULL) &&
> + (AcpiTableInfo != NULL) &&
> + (CfgMgrProtocol != NULL) &&
> + (Table != NULL) &&
> + (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
> + (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
> + );
> +
> + DEBUG ((
> + DEBUG_ERROR,
> + "DEBUG PRINT: MPAM: Requested table revision = %d\n",
> + AcpiTableInfo->AcpiTableRevision
> + ));
> +
> + if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
> + (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: 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;
> + }
> +
> + Generator = (ACPI_MPAM_GENERATOR *)This;
> + *Table = NULL;
> +
> + // Get the Memory System Controller Node info and update the MPAM //
> + structure count with MSC Node count (Type 0) Status =
> + GetEArmObjMscNodeInfo (
> + CfgMgrProtocol,
> + CM_NULL_TOKEN,
> + &MscNodeList,
> + &MscNodeCount
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to get memory system controller node info.
> Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + MpamNodeCount = MscNodeCount;
> + Generator->MscNodeCount = MscNodeCount;
> +
> + // Allocate Node Indexer array
> + NodeIndexer = (MPAM_NODE_INDEXER *)AllocateZeroPool (
> + sizeof (MPAM_NODE_INDEXER) *
> + MpamNodeCount
> + ); if (NodeIndexer == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to allocate memory for Node Indexer. Status =
> %r\n ",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + DEBUG ((DEBUG_INFO, "MPAM INFO: NodeIndexer = %p\n",
> NodeIndexer));
> + Generator->MpamNodeCount = MpamNodeCount;
> + Generator->NodeIndexer = NodeIndexer;
> +
> + // Calculate the size of the MPAM table TableSize = sizeof
> +
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEA
> + DER);
> +
> + // Include the size of MSC Nodes and index them if
> + (Generator->MscNodeCount != 0) {
> + MscNodeOffset = TableSize;
> + // Size of MSC nodes.
> + NodeSize = GetSizeofMscGroupNodes (
> + MscNodeOffset,
> + MscNodeList,
> + Generator->MscNodeCount,
> + &NodeIndexer
> + );
[Rohit] MscNodeOffset is sizeof (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER), which moves past EFI_ACPI_DESCRIPTION_HEADER by 12 bytes. Should we be having these fields in the table type?
UINT32 NumNodes;
UINT32 NodeOffset;
UINT32 Reserved;
I had added a similar comment on [PATCH 1/2].
[/Rohit]
> + if (NodeSize > MAX_UINT32) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Invalid Size of Group Nodes. Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + TableSize += NodeSize;
> +
> + DEBUG ((
> + DEBUG_INFO,
> + " MscNodeCount = %d\n" \
> + " MscNodeOffset = 0x%x\n",
> + Generator->MscNodeCount,
> + MscNodeOffset
> + ));
> + }
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "INFO: MPAM:\n" \
> + " MpamNodeCount = %d\n" \
> + " TableSize = 0x%X\n",
> + MpamNodeCount,
> + TableSize
> + ));
> +
> + if (TableSize > MAX_UINT32) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: MPAM Table Size 0x%lx > MAX_UINT32," \
> + " Status = %r\n",
> + TableSize,
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + // Allocate the Buffer for the MPAM table *Table =
> + (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize); if
> + (*Table == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to allocate memory for MPAM Table. " \
> + "Size = %d. Status = %r\n",
> + TableSize,
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + Mpam =
> +
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEA
> + DER *)*Table;
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "MPAM: Mpam = 0x%p. TableSize = 0x%x\n",
> + Mpam,
> + TableSize
> + ));
> +
> + // Add ACPI header
> + Status = AddAcpiHeader (
> + CfgMgrProtocol,
> + This,
> + &Mpam->Header,
> + AcpiTableInfo,
> + TableSize
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to add ACPI header. Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> + // Update MPAM table
> + Mpam->NumNodes = MscNodeCount;
> + Mpam->NodeOffset = sizeof
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEADER);
> + Mpam->Reserved = EFI_ACPI_RESERVED_DWORD;
> +
> + // Add MSC Nodes to the generated table if (Mpam->NumNodes != 0) {
> + Status = AddMscNodes (
> + This,
> + CfgMgrProtocol,
> + Mpam,
> + MscNodeOffset,
> + MscNodeList,
> + MscNodeCount
> + );
[Rohit] Same comment as above
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: MPAM: Failed to add MSC Nodes. Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> + }
> +
> + return Status;
> +
> +error_handler:
> + if (Generator->NodeIndexer != NULL) {
> + FreePool (Generator->NodeIndexer);
> + Generator->NodeIndexer = NULL;
> + }
> +
> + if (*Table != NULL) {
> + FreePool (*Table);
> + *Table = NULL;
> + }
> +
> + return Status;
> +}
> +
> +/** Free any resources allocated for constructing the MPAM
[Rohit] Should this be "constructing the MPAM table"?
> +
> + @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
> +FreeMpamTableResources (
> + 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
> + )
> +{
> + ACPI_MPAM_GENERATOR *Generator;
> +
> + ASSERT (This != NULL);
> + ASSERT (AcpiTableInfo != NULL);
> + ASSERT (CfgMgrProtocol != NULL);
> + ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
> + ASSERT (AcpiTableInfo->AcpiTableSignature ==
> + This->AcpiTableSignature);
> +
> + Generator = (ACPI_MPAM_GENERATOR *)This;
> +
> + // Free any memory allocated by the generator if
> + (Generator->NodeIndexer != NULL) {
> + FreePool (Generator->NodeIndexer);
> + Generator->NodeIndexer = NULL;
> + }
> +
> + if ((Table == NULL) || (*Table == NULL)) {
> + DEBUG ((DEBUG_ERROR, "ERROR: MPAM: Invalid Table Pointer\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + FreePool (*Table);
> + *Table = NULL;
> +
> + return EFI_SUCCESS;
> +}
> +
> +/** The MPAM Table Generator revision.
> +*/
> +#define MPAM_GENERATOR_REVISION CREATE_REVISION (1, 0)
> +
> +/** The interface for the MPAM Table Generator.
> +*/
> +STATIC
> +ACPI_MPAM_GENERATOR MpamGenerator = {
> + // ACPI table generator header
> + {
> + // Generator ID
> + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMpam),
> + // Generator Description
> + L"ACPI.STD.MPAM.GENERATOR",
> + // ACPI Table Signature
> +
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_STRUCTURE_SIGNATURE,
> + // ACPI Table Revision supported by this Generator
> +
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_REVISION,
> + // Minimum supported ACPI Table Revision
> +
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_REVISION,
> + // Creator ID
> + TABLE_GENERATOR_CREATOR_ID_ARM,
> + // Creator Revision
> + MPAM_GENERATOR_REVISION,
> + // Build Table function
> + BuildMpamTable,
> + // Free Resource function
> + FreeMpamTableResources,
> + // Extended build function not needed
> + NULL,
> + // Extended build function not implemented by the generator.
> + // Hence extended free resource function is not required.
> + NULL
> + },
> +
> + // MPAM Generator private data
> +
> + // MPAM node count
> + 0,
> + // MSC node count
> + 0,
> +
> + // Pointer to MPAM Node Indexer
> + 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
> +AcpiMpamLibConstructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = RegisterAcpiTableGenerator (&MpamGenerator.Header);
> + DEBUG ((DEBUG_INFO, "MPAM: 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
> +AcpiMpamLibDestructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = DeregisterAcpiTableGenerator (&MpamGenerator.Header);
> + DEBUG ((DEBUG_INFO, "MPAM: Deregister Generator. Status = %r\n",
> +Status));
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> +}
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> h
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> h
> new file mode 100644
> index 0000000000..1075bd8c6c
> --- /dev/null
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> h
> @@ -0,0 +1,47 @@
> +/** @file
> + Header file for the dynamic MPAM generator
> +
> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2022, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Reference(s):
> + - ACPI 6.4 Specification, January 2021
> + - ARM Architecture Reference Manual ARMv8
> +
> + @par Glossary:
> + - Cm or CM - Configuration Manager
> + - Obj or OBJ - Object
> +**/
> +
> +#ifndef MPAM_GENERATOR_H_
> +#define MPAM_GENERATOR_H_
> +
> +#pragma pack(1)
> +
> +/** A structure that describes the Node indexer
> + used for indexing the MPAM MSC nodes.
> +*/
> +typedef struct MpamNodeIndexer {
> + /// Index token for the Node
> + CM_OBJECT_TOKEN Token;
> + /// Pointer to the node
> + VOID *Object;
> + /// Node offset from the start of the MPAM table
> + UINT32 Offset;
> +} MPAM_NODE_INDEXER;
> +
> +typedef struct AcpiMpamGenerator {
> + /// ACPI Table generator header
> + ACPI_TABLE_GENERATOR Header;
> + /// MPAM structure count
> + UINT32 MpamNodeCount;
> + /// Count of Msc Nodes
> + UINT32 MscNodeCount;
> + /// List of indexed CM objects for MPAM generation
> + MPAM_NODE_INDEXER *NodeIndexer;
> +} ACPI_MPAM_GENERATOR;
> +
> +#pragma pack()
> +
> +#endif // MPAM_GENERATOR_H_
> --
> 2.17.1
>
>
>
>
>
Regards,
Rohit
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
2022-08-19 8:26 ` [edk2-devel] " Rohit Mathew
@ 2022-08-23 17:51 ` Swatisri Kantamsetti
1 sibling, 0 replies; 15+ messages in thread
From: Swatisri Kantamsetti @ 2022-08-23 17:51 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami.Mujawar@arm.com,
Alexei.Fedorov@arm.com, michael.d.kinney@intel.com,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com
Cc: Jeff Brasen
[-- Attachment #1: Type: text/plain, Size: 30843 bytes --]
Hello,
Just a reminder to provide feedback on this patch.
Thanks,
Swati
swatisrik@nvidia.com
________________________________
From: Name <username@nvidia.com>
Sent: Tuesday, August 16, 2022 2:18 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Sami.Mujawar@arm.com <Sami.Mujawar@arm.com>; Alexei.Fedorov@arm.com <Alexei.Fedorov@arm.com>; michael.d.kinney@intel.com <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <zhiguang.liu@intel.com>
Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
Subject: [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files
From: Swatisri Kantamsetti <swatisrik@nvidia.com>
ACPI header, MSC and Resource Nodes are populated
in the MPAM Table
Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
---
DynamicTablesPkg/DynamicTables.dsc.inc | 2 +
DynamicTablesPkg/Include/AcpiTableGenerator.h | 1 +
.../Include/ArmNameSpaceObjects.h | 68 ++
.../Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf | 30 +
.../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c | 649 ++++++++++++++++++
.../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h | 47 ++
6 files changed, 797 insertions(+)
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
create mode 100644 DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 3d4fa0c4c4..745d5f0633 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -29,6 +29,7 @@
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
+ DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
@@ -54,6 +55,7 @@
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.inf
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index f962dbff57..56d7375b4a 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -94,6 +94,7 @@ typedef enum StdAcpiTableId {
EStdAcpiTableIdIort, ///< IORT Generator
EStdAcpiTableIdPptt, ///< PPTT Generator
EStdAcpiTableIdSrat, ///< SRAT Generator
+ EStdAcpiTableIdMpam, ///< MPAM Generator
EStdAcpiTableIdSsdtSerialPort, ///< SSDT Serial-Port Generator
EStdAcpiTableIdSsdtCmn600, ///< SSDT Cmn-600 Generator
EStdAcpiTableIdSsdtCpuTopology, ///< SSDT Cpu Topology
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 102e0f96be..39a14ed0b3 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -63,6 +63,8 @@ typedef enum ArmObjectID {
EArmObjPciInterruptMapInfo, ///< 39 - Pci Interrupt Map Info
EArmObjRmr, ///< 40 - Reserved Memory Range Node
EArmObjMemoryRangeDescriptor, ///< 41 - Memory Range Descriptor
+ EArmObjMscNodeInfo, ///< 40 - Msc Memory System Controller Node Info
+ EArmObjResNodeInfo, ///< 41 - Res Resource Node Info
EArmObjMax
} EARM_OBJECT_ID;
@@ -1070,6 +1072,72 @@ typedef struct CmArmRmrDescriptor {
UINT64 Length;
} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
+/** A structure that describes Memory System Controller Node.
+
+ MPAM Memory System Component Nodes are described by
+ this object.
+
+ ID: EArmObjMscNodeInfo
+*/
+typedef struct CmArmMscNodeInfo {
+ /// An unique token used to identify this object
+ CM_OBJECT_TOKEN Token;
+
+ /// MPAM Base Address
+ UINT64 BaseAddress;
+ /// MMIO Size
+ UINT32 MmioSize;
+ /// Overflow Interrupt
+ UINT32 OverflowInterrupt;
+ /// Overflow Interrupt Flags
+ UINT32 OverflowInterruptFlags;
+ /// Overflow Interrupt Affinity
+ UINT32 OverflowInterruptAff;
+ /// Error Interrupt
+ UINT32 ErrorInterrupt;
+ /// Error Interrupt Flags
+ UINT32 ErrorInterruptFlags;
+ /// Error Interrupt Affinity
+ UINT32 ErrorInterruptAff;
+ /// Not Ready Signal time
+ UINT32 MaxNRdyUsec;
+ /// Linked Device HWID
+ UINT64 LinkedDeviceHwId;
+ /// Linked Device Instance ID
+ UINT32 LinkedDeviceInstanceHwId;
+ /// Number of Resource nodes
+ UINT32 NumResourceNodes;
+ /// Reference token for the list of resource nodes
+ //CM_OBJECT_TOKEN ResourceNodeListToken;
+
+} CM_ARM_MSC_NODE_INFO;
+
+/** A structure that describes Memory System Controller Node.
+
+ MPAM Memory System Component Nodes are described by
+ this object.
+
+ ID: EArmObjResNodeInfo
+*/
+typedef struct CmArmResNodeInfo {
+ /// An unique token used to identify this object
+ CM_OBJECT_TOKEN Token;
+
+ /// Identifier
+ UINT32 Identifier;
+ /// RIS Index
+ UINT8 RisIndex;
+ /// Locator Type
+ UINT8 LocatorType;
+ /// Locator
+ UINT64 Locator;
+ /// Num functional dependencies
+ UINT32 NumFuncDep;
+ /// Reference token for the list of resource nodes
+ CM_OBJECT_TOKEN FuncDepListToken;
+
+} CM_ARM_RESOURCE_NODE_INFO;
+
#pragma pack()
#endif // ARM_NAMESPACE_OBJECTS_H_
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
new file mode 100644
index 0000000000..480130dc21
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf
@@ -0,0 +1,30 @@
+## @file
+# MPAM Table Generator Inf file
+#
+# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+# Copyright (c) 2022, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = AcpiMpamLibArm
+ FILE_GUID = 02d0c79f-41cd-45c9-9835-781229c619d1
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = NULL|DXE_DRIVER
+ CONSTRUCTOR = AcpiMpamLibConstructor
+ DESTRUCTOR = AcpiMpamLibDestructor
+
+[Sources]
+ MpamGenerator.c
+ MpamGenerator.h
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ DynamicTablesPkg/DynamicTablesPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
new file mode 100644
index 0000000000..db3e8e95bc
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
@@ -0,0 +1,649 @@
+/** @file
+ MPAM Table Generator
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.4 Specification, January 2021
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#include <IndustryStandard/Mpam.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>
+
+#include "MpamGenerator.h"
+
+/**
+ ARM standard MPAM Generator
+
+ Requirements:
+ The following Configuration Manager Object(s) are used by this Generator:
+ - EArmObjMscNodeInfo (REQUIRED)
+ - EArmObjResNodeInfo
+*/
+
+/**
+ This macro expands to a function that retrieves the MSC Node information
+ from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceArm,
+ EArmObjMscNodeInfo,
+ CM_ARM_MSC_NODE_INFO
+ );
+
+/**
+ This macro expands to a function that retrieves the Resource Node
+ information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+ EObjNameSpaceArm,
+ EArmObjResNodeInfo,
+ CM_ARM_RESOURCE_NODE_INFO
+ );
+
+/**
+ Returns the size of the MPAM Memory System Controller (MSC) Node
+
+ @param [in] Node Pointer to MSC Node Info CM object
+
+ @retval Size of the MSC Node in bytes.
+**/
+STATIC
+UINT32
+GetMscNodeSize (
+ IN CONST CM_ARM_MSC_NODE_INFO *Node
+ )
+{
+ ASSERT (Node != NULL);
+
+ // <size of Memory System Controller Node>
+ return (UINT32)(sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE) +
+ Node->NumResourceNodes * sizeof (EFI_ACPI_6_4_MPAM_RESOURCE_NODE));
+}
+
+/** Returns the total size required for the MSC and
+ updates the Node Indexer.
+
+ This function calculates the size required for the node group
+ and also populates the Node Indexer array with offsets for the
+ individual nodes.
+
+ @param [in] NodeStartOffset Offset from the start of the
+ MPAM where this node group starts.
+ @param [in] NodeList Pointer to MSC Group node list.
+ @param [in] NodeCount Count of the MSC Group nodes.
+ @param [in, out] NodeIndexer Pointer to the next Node Indexer.
+
+ @retval Total size of the ITS Group Nodes.
+**/
+STATIC
+UINT64
+GetSizeofMscGroupNodes (
+ IN CONST UINT32 NodeStartOffset,
+ IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
+ IN UINT32 NodeCount,
+ IN OUT MPAM_NODE_INDEXER **CONST NodeIndexer
+ )
+{
+ UINT64 Size;
+
+ ASSERT (NodeList != NULL);
+
+ Size = 0;
+ while (NodeCount-- != 0) {
+ (*NodeIndexer)->Token = NodeList->Token;
+ (*NodeIndexer)->Object = (VOID *)NodeList;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+ DEBUG ((
+ DEBUG_INFO,
+ "MPAM: MSC Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+ *NodeIndexer,
+ (*NodeIndexer)->Token,
+ (*NodeIndexer)->Object,
+ (*NodeIndexer)->Offset
+ ));
+
+ Size += GetMscNodeSize (NodeList);
+
+ (*NodeIndexer)++;
+ NodeList++;
+ }
+
+ return Size;
+}
+
+/** Update the MSC Group Node Information.
+
+ @param [in] This Pointer to the table Generator.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in] Mpam Pointer to MPAM table structure.
+ @param [in] NodesStartOffset Offset for the start of the Msc Group
+ Nodes.
+ @param [in] NodeList Pointer to an array of Msc Group Node
+ Objects.
+ @param [in] NodeCount Number of Msc Group Node Objects.
+
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object was not found.
+**/
+STATIC
+EFI_STATUS
+AddMscNodes (
+ IN CONST ACPI_TABLE_GENERATOR *CONST This,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *Mpam,
+ IN CONST UINT32 NodesStartOffset,
+ IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
+ IN UINT32 NodeCount
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_6_4_MPAM_MSC_NODE *MscNode;
+ EFI_ACPI_6_4_MPAM_RESOURCE_NODE *ResourceNodeArray;
+ CM_ARM_RESOURCE_NODE_INFO *ResourceNodeList;
+ UINT64 NodeLength;
+ UINT32 ResourceNodeCount;
+
+ ASSERT (Mpam != NULL);
+
+ MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)Mpam + NodesStartOffset);
+
+ // Get the Resource Node info to update the MPAM MSC node
+ Status = GetEArmObjResNodeInfo (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &ResourceNodeList,
+ &ResourceNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add resource nodes info. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ while (NodeCount-- != 0) {
+ NodeLength = GetMscNodeSize (NodeList);
+
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: MSC Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
+ // Populate the node header
+ MscNode->Length = (UINT16)NodeLength;
+ MscNode->Reserved = EFI_ACPI_RESERVED_WORD;
+ MscNode->Identifier = EFI_ACPI_RESERVED_DWORD;
+ MscNode->BaseAddress = NodeList->BaseAddress;
+ MscNode->MmioSize = NodeList->MmioSize;
+ MscNode->OverflowInterrupt = NodeList->OverflowInterrupt;
+ MscNode->OverflowInterruptFlags = NodeList->OverflowInterruptFlags;
+ MscNode->Reserved1 = EFI_ACPI_RESERVED_DWORD;
+ MscNode->OverflowInterruptAff = NodeList->OverflowInterruptAff;
+ MscNode->ErrorInterrupt = NodeList->ErrorInterrupt;
+ MscNode->ErrorInterruptFlags = NodeList->ErrorInterruptFlags;
+ MscNode->Reserved2 = EFI_ACPI_RESERVED_DWORD;
+ MscNode->ErrorInterruptAff = NodeList->ErrorInterruptAff;
+ MscNode->MaxNRdyUsec = NodeList->MaxNRdyUsec;
+ MscNode->LinkedDeviceHwId = NodeList->LinkedDeviceHwId;
+ MscNode->LinkedDeviceInstanceHwId = NodeList->LinkedDeviceInstanceHwId;
+ MscNode->NumResourceNodes = NodeList->NumResourceNodes;
+
+ // ResourceNode List for each MSC
+ if (MscNode->NumResourceNodes > 0) {
+ // Resource Node array for this Msc node
+ ResourceNodeArray = (EFI_ACPI_6_4_MPAM_RESOURCE_NODE *)((UINT8 *)MscNode + sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE));
+ // Adding Resource Node content
+ while ( MscNode->NumResourceNodes-- != 0 ) {
+ ResourceNodeArray->Identifier = ResourceNodeList->Identifier;
+ ResourceNodeArray->RisIndex = ResourceNodeList->RisIndex;
+ ResourceNodeArray->Reserved1 = EFI_ACPI_RESERVED_WORD;
+ ResourceNodeArray->LocatorType = ResourceNodeList->LocatorType;
+ ResourceNodeArray->Locator = ResourceNodeList->Locator;
+ ResourceNodeArray->NumFuncDep = ResourceNodeList->NumFuncDep;
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add Resource Node List. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ ResourceNodeList++;
+ ResourceNodeArray++;
+ }
+ }
+
+ // Next MSC Node
+ MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)MscNode + MscNode->Length);
+ NodeList++;
+ } // Msc Node
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Construct the MPAM ACPI table.
+
+ 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 generator to be used.
+ @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.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildMpamTable (
+ 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;
+ UINT64 TableSize;
+ UINT64 NodeSize;
+ UINT32 MpamNodeCount;
+
+ UINT32 MscNodeCount;
+ UINT32 MscNodeOffset;
+
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *Mpam;
+ ACPI_MPAM_GENERATOR *Generator;
+ CM_ARM_MSC_NODE_INFO *MscNodeList;
+ MPAM_NODE_INDEXER *NodeIndexer;
+
+ ASSERT (
+ (This != NULL) &&
+ (AcpiTableInfo != NULL) &&
+ (CfgMgrProtocol != NULL) &&
+ (Table != NULL) &&
+ (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
+ (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
+ );
+
+ DEBUG ((
+ DEBUG_ERROR,
+ "DEBUG PRINT: MPAM: Requested table revision = %d\n",
+ AcpiTableInfo->AcpiTableRevision
+ ));
+
+ if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
+ (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision))
+ {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: 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;
+ }
+
+ Generator = (ACPI_MPAM_GENERATOR *)This;
+ *Table = NULL;
+
+ // Get the Memory System Controller Node info and update the MPAM
+ // structure count with MSC Node count (Type 0)
+ Status = GetEArmObjMscNodeInfo (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &MscNodeList,
+ &MscNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to get memory system controller node info. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ MpamNodeCount = MscNodeCount;
+ Generator->MscNodeCount = MscNodeCount;
+
+ // Allocate Node Indexer array
+ NodeIndexer = (MPAM_NODE_INDEXER *)AllocateZeroPool (
+ sizeof (MPAM_NODE_INDEXER) *
+ MpamNodeCount
+ );
+ if (NodeIndexer == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to allocate memory for Node Indexer. Status = %r\n ",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ DEBUG ((DEBUG_INFO, "MPAM INFO: NodeIndexer = %p\n", NodeIndexer));
+ Generator->MpamNodeCount = MpamNodeCount;
+ Generator->NodeIndexer = NodeIndexer;
+
+ // Calculate the size of the MPAM table
+ TableSize = sizeof (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER);
+
+ // Include the size of MSC Nodes and index them
+ if (Generator->MscNodeCount != 0) {
+ MscNodeOffset = TableSize;
+ // Size of MSC nodes.
+ NodeSize = GetSizeofMscGroupNodes (
+ MscNodeOffset,
+ MscNodeList,
+ Generator->MscNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Invalid Size of Group Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " MscNodeCount = %d\n" \
+ " MscNodeOffset = 0x%x\n",
+ Generator->MscNodeCount,
+ MscNodeOffset
+ ));
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "INFO: MPAM:\n" \
+ " MpamNodeCount = %d\n" \
+ " TableSize = 0x%X\n",
+ MpamNodeCount,
+ TableSize
+ ));
+
+ if (TableSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: MPAM Table Size 0x%lx > MAX_UINT32," \
+ " Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
+ // Allocate the Buffer for the MPAM table
+ *Table = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize);
+ if (*Table == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to allocate memory for MPAM Table. " \
+ "Size = %d. Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
+ Mpam = (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER *)*Table;
+
+ DEBUG ((
+ DEBUG_INFO,
+ "MPAM: Mpam = 0x%p. TableSize = 0x%x\n",
+ Mpam,
+ TableSize
+ ));
+
+ // Add ACPI header
+ Status = AddAcpiHeader (
+ CfgMgrProtocol,
+ This,
+ &Mpam->Header,
+ AcpiTableInfo,
+ TableSize
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add ACPI header. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
+ // Update MPAM table
+ Mpam->NumNodes = MscNodeCount;
+ Mpam->NodeOffset = sizeof (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER);
+ Mpam->Reserved = EFI_ACPI_RESERVED_DWORD;
+
+ // Add MSC Nodes to the generated table
+ if (Mpam->NumNodes != 0) {
+ Status = AddMscNodes (
+ This,
+ CfgMgrProtocol,
+ Mpam,
+ MscNodeOffset,
+ MscNodeList,
+ MscNodeCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MPAM: Failed to add MSC Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ }
+
+ return Status;
+
+error_handler:
+ if (Generator->NodeIndexer != NULL) {
+ FreePool (Generator->NodeIndexer);
+ Generator->NodeIndexer = NULL;
+ }
+
+ if (*Table != NULL) {
+ FreePool (*Table);
+ *Table = NULL;
+ }
+
+ return Status;
+}
+
+/** Free any resources allocated for constructing the MPAM
+
+ @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
+FreeMpamTableResources (
+ 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
+ )
+{
+ ACPI_MPAM_GENERATOR *Generator;
+
+ ASSERT (This != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
+ ASSERT (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature);
+
+ Generator = (ACPI_MPAM_GENERATOR *)This;
+
+ // Free any memory allocated by the generator
+ if (Generator->NodeIndexer != NULL) {
+ FreePool (Generator->NodeIndexer);
+ Generator->NodeIndexer = NULL;
+ }
+
+ if ((Table == NULL) || (*Table == NULL)) {
+ DEBUG ((DEBUG_ERROR, "ERROR: MPAM: Invalid Table Pointer\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ FreePool (*Table);
+ *Table = NULL;
+
+ return EFI_SUCCESS;
+}
+
+/** The MPAM Table Generator revision.
+*/
+#define MPAM_GENERATOR_REVISION CREATE_REVISION (1, 0)
+
+/** The interface for the MPAM Table Generator.
+*/
+STATIC
+ACPI_MPAM_GENERATOR MpamGenerator = {
+ // ACPI table generator header
+ {
+ // Generator ID
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMpam),
+ // Generator Description
+ L"ACPI.STD.MPAM.GENERATOR",
+ // ACPI Table Signature
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_STRUCTURE_SIGNATURE,
+ // ACPI Table Revision supported by this Generator
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION,
+ // Minimum supported ACPI Table Revision
+ EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION,
+ // Creator ID
+ TABLE_GENERATOR_CREATOR_ID_ARM,
+ // Creator Revision
+ MPAM_GENERATOR_REVISION,
+ // Build Table function
+ BuildMpamTable,
+ // Free Resource function
+ FreeMpamTableResources,
+ // Extended build function not needed
+ NULL,
+ // Extended build function not implemented by the generator.
+ // Hence extended free resource function is not required.
+ NULL
+ },
+
+ // MPAM Generator private data
+
+ // MPAM node count
+ 0,
+ // MSC node count
+ 0,
+
+ // Pointer to MPAM Node Indexer
+ 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
+AcpiMpamLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = RegisterAcpiTableGenerator (&MpamGenerator.Header);
+ DEBUG ((DEBUG_INFO, "MPAM: 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
+AcpiMpamLibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = DeregisterAcpiTableGenerator (&MpamGenerator.Header);
+ DEBUG ((DEBUG_INFO, "MPAM: Deregister Generator. Status = %r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
new file mode 100644
index 0000000000..1075bd8c6c
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
@@ -0,0 +1,47 @@
+/** @file
+ Header file for the dynamic MPAM generator
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.4 Specification, January 2021
+ - ARM Architecture Reference Manual ARMv8
+
+ @par Glossary:
+ - Cm or CM - Configuration Manager
+ - Obj or OBJ - Object
+**/
+
+#ifndef MPAM_GENERATOR_H_
+#define MPAM_GENERATOR_H_
+
+#pragma pack(1)
+
+/** A structure that describes the Node indexer
+ used for indexing the MPAM MSC nodes.
+*/
+typedef struct MpamNodeIndexer {
+ /// Index token for the Node
+ CM_OBJECT_TOKEN Token;
+ /// Pointer to the node
+ VOID *Object;
+ /// Node offset from the start of the MPAM table
+ UINT32 Offset;
+} MPAM_NODE_INDEXER;
+
+typedef struct AcpiMpamGenerator {
+ /// ACPI Table generator header
+ ACPI_TABLE_GENERATOR Header;
+ /// MPAM structure count
+ UINT32 MpamNodeCount;
+ /// Count of Msc Nodes
+ UINT32 MscNodeCount;
+ /// List of indexed CM objects for MPAM generation
+ MPAM_NODE_INDEXER *NodeIndexer;
+} ACPI_MPAM_GENERATOR;
+
+#pragma pack()
+
+#endif // MPAM_GENERATOR_H_
--
2.17.1
[-- Attachment #2: Type: text/html, Size: 57586 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-16 20:18 [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Name
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
2022-08-19 8:26 ` [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Rohit Mathew
@ 2022-08-23 17:52 ` Swatisri Kantamsetti
2022-10-03 11:17 ` Sami Mujawar
3 siblings, 0 replies; 15+ messages in thread
From: Swatisri Kantamsetti @ 2022-08-23 17:52 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami.Mujawar@arm.com,
Alexei.Fedorov@arm.com, michael.d.kinney@intel.com,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com
Cc: Jeff Brasen
[-- Attachment #1: Type: text/plain, Size: 3718 bytes --]
Just a reminder to provide feedback on this patch.
Thanks,
Swati
swatisrik@nvidia.com
________________________________
From: Name <username@nvidia.com>
Sent: Tuesday, August 16, 2022 2:18 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Sami.Mujawar@arm.com <Sami.Mujawar@arm.com>; Alexei.Fedorov@arm.com <Alexei.Fedorov@arm.com>; michael.d.kinney@intel.com <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <zhiguang.liu@intel.com>
Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
Subject: [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
From: Swatisri Kantamsetti <swatisrik@nvidia.com>
Added MPAM table header, MSC and Resource Node
info structures
Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
---
MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
MdePkg/Include/IndustryStandard/Mpam.h | 69 ++++++++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
index fe5ebfac2b..e54f631186 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2952,6 +2952,11 @@ typedef struct {
///
#define EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('P', 'P', 'T', 'T')
+///
+/// "MPAM" Memory System Resource Partitioning And Monitoring Table
+///
+#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
+
///
/// "PSDT" Persistent System Description Table
///
diff --git a/MdePkg/Include/IndustryStandard/Mpam.h b/MdePkg/Include/IndustryStandard/Mpam.h
new file mode 100644
index 0000000000..e0f75f0114
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Mpam.h
@@ -0,0 +1,69 @@
+/** @file
+ ACPI Memory System Resource Partitioning And Monitoring (MPAM)
+ as specified in ARM spec DEN0065
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef _MPAM_H_
+#define _MPAM_H_
+
+#pragma pack(1)
+
+///
+/// Memory System Resource Partitioning and Monitoring Table (MPAM)
+///
+typedef struct {
+ EFI_ACPI_DESCRIPTION_HEADER Header;
+ UINT32 NumNodes;
+ UINT32 NodeOffset;
+ UINT32 Reserved;
+} EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER;
+
+///
+/// MPAM Revision (as defined in ACPI 6.4 spec.)
+///
+#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION 0x01
+
+///
+/// Memory System Controller Node Structure
+///
+
+typedef struct {
+ UINT16 Length;
+ UINT16 Reserved;
+ UINT32 Identifier;
+ UINT64 BaseAddress;
+ UINT32 MmioSize;
+ UINT32 OverflowInterrupt;
+ UINT32 OverflowInterruptFlags;
+ UINT32 Reserved1;
+ UINT32 OverflowInterruptAff;
+ UINT32 ErrorInterrupt;
+ UINT32 ErrorInterruptFlags;
+ UINT32 Reserved2;
+ UINT32 ErrorInterruptAff;
+ UINT32 MaxNRdyUsec;
+ UINT64 LinkedDeviceHwId;
+ UINT32 LinkedDeviceInstanceHwId;
+ UINT32 NumResourceNodes;
+} EFI_ACPI_6_4_MPAM_MSC_NODE;
+
+///
+/// Resource Node Structure
+///
+
+typedef struct {
+ UINT32 Identifier;
+ UINT8 RisIndex;
+ UINT16 Reserved1;
+ UINT8 LocatorType;
+ UINT64 Locator;
+ UINT32 NumFuncDep;
+} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
+
+#pragma pack()
+
+#endif
--
2.17.1
[-- Attachment #2: Type: text/html, Size: 6216 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-19 8:26 ` [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Rohit Mathew
@ 2022-08-23 20:10 ` Andrew Fish
2022-08-23 21:28 ` Rohit Mathew
2022-08-24 14:39 ` hesham.almatary
0 siblings, 2 replies; 15+ messages in thread
From: Andrew Fish @ 2022-08-23 20:10 UTC (permalink / raw)
To: devel, rohit.mathew
Cc: username@nvidia.com, Sami Mujawar, Alexei Fedorov, Mike Kinney,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com,
Swatisri Kantamsetti, Thomas Abraham, Thanu Rangarajan, nd
[-- Attachment #1: Type: text/plain, Size: 6150 bytes --]
> On Aug 19, 2022, at 1:26 AM, Rohit Mathew <rohit.mathew@arm.com> wrote:
>
> Hi Swatisri,
>
> Thanks for the patch. Please find my comments inline marked [Rohit] -
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Name
>> via groups.io <http://groups.io/>
>> Sent: 16 August 2022 21:18
>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>;
>> Alexei Fedorov <Alexei.Fedorov@arm.com <mailto:Alexei.Fedorov@arm.com>>; michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>;
>> gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>
>> Cc: Swatisri Kantamsetti <swatisrik@nvidia.com <mailto:swatisrik@nvidia.com>>
>> Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
>>
>> From: Swatisri Kantamsetti <swatisrik@nvidia.com>
>>
>> Added MPAM table header, MSC and Resource Node info structures
>>
>> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
>> ---
>> MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
>> MdePkg/Include/IndustryStandard/Mpam.h | 69
>> ++++++++++++++++++++++++
>> 2 files changed, 74 insertions(+)
>> create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
>>
>> diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
>> b/MdePkg/Include/IndustryStandard/Acpi64.h
>> index fe5ebfac2b..e54f631186 100644
>> --- a/MdePkg/Include/IndustryStandard/Acpi64.h
>> +++ b/MdePkg/Include/IndustryStandard/Acpi64.h
>> @@ -2952,6 +2952,11 @@ typedef struct {
>> ///
>> #define
>> EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SI
>> GNATURE SIGNATURE_32('P', 'P', 'T', 'T')
>>
>> +///
>> +/// "MPAM" Memory System Resource Partitioning And Monitoring Table
>> ///
>> +#define
>> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
>> NG_TABLE_STRUC
>> +TURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
>> +
>> ///
>> /// "PSDT" Persistent System Description Table /// diff --git
>> a/MdePkg/Include/IndustryStandard/Mpam.h
>> b/MdePkg/Include/IndustryStandard/Mpam.h
>> new file mode 100644
>> index 0000000000..e0f75f0114
>> --- /dev/null
>> +++ b/MdePkg/Include/IndustryStandard/Mpam.h
>> @@ -0,0 +1,69 @@
>> +/** @file
>> + ACPI Memory System Resource Partitioning And Monitoring (MPAM)
>> + as specified in ARM spec DEN0065
>> +
>> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
>> + Copyright (c) 2022, ARM Limited. All rights reserved.
>> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
>> +
>> +#ifndef _MPAM_H_
>> +#define _MPAM_H_
>> +
>> +#pragma pack(1)
>> +
>> +///
>> +/// Memory System Resource Partitioning and Monitoring Table (MPAM)
>> ///
>> +typedef struct {
>> + EFI_ACPI_DESCRIPTION_HEADER Header;
>> + UINT32 NumNodes;
>> + UINT32 NodeOffset;
>> + UINT32 Reserved;
>> +}
>> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
>> NG_TABLE_HEADE
>> +R;
>
> [Rohit] Shouldn't the header be followed by MSC node body type as defined in MPAM ACPI 1.0, section 2, table 3 - The MPAM table and section 2.1, table 4 - MSC Node body?
>
>> +
>> +///
>> +/// MPAM Revision (as defined in ACPI 6.4 spec.) /// #define
>> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
>> NG_TABLE_REVIS
>> +ION 0x01
>> +
>> +///
>> +/// Memory System Controller Node Structure ///
>> +
>> +typedef struct {
>> + UINT16 Length;
>> + UINT16 Reserved;
>> + UINT32 Identifier;
>> + UINT64 BaseAddress;
>> + UINT32 MmioSize;
>> + UINT32 OverflowInterrupt;
>> + UINT32 OverflowInterruptFlags;
>
> [Rohit] Would it be better to have a type (possibly bitfield struct) instead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - Interrupt flags)
>
Probably better NOT to use bitfields in APIs that are produced and consumed by different worlds. While the the UEFI does speak to the bit order of or a bitfield the rules around packing of bitfields is compiler defined.
I just saw a bug last week with bitfield compatibility that was introduced by clang fixing its -mms-bitfields implementation. The GCC rules for packing bitfields is different than VC++ so that is why the compiler flag -mms-bitfields exists in the 1st place . A clang -mms-bitfields bug got fixed and it broke the code as the extra padding required by VC++ got added to the bitfield.
Thanks,
Andrew Fish
>> + UINT32 Reserved1;
>> + UINT32 OverflowInterruptAff;
>> + UINT32 ErrorInterrupt;
>> + UINT32 ErrorInterruptFlags;
>
> [Rohit ] Same comment as before above.
>
>> + UINT32 Reserved2;
>> + UINT32 ErrorInterruptAff;
>> + UINT32 MaxNRdyUsec;
>> + UINT64 LinkedDeviceHwId;
>> + UINT32 LinkedDeviceInstanceHwId;
>> + UINT32 NumResourceNodes;
>> +} EFI_ACPI_6_4_MPAM_MSC_NODE;
>> +
>> +///
>> +/// Resource Node Structure
>> +///
>> +
>> +typedef struct {
>> + UINT32 Identifier;
>> + UINT8 RisIndex;
>> + UINT16 Reserved1;
>> + UINT8 LocatorType;
>> + UINT64 Locator;
>
> [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
>
>> + UINT32 NumFuncDep;
>> +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
>
> [Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NODE type and this could be non-zero, should we also need the type for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Functional dependency descriptor)
>
> [Rohit] Also, could some of the commonly used macros be added to this header, please? (location types, MPAM interrupt mode, interrupt types, affinity type, etc)
>
>> +
>> +#pragma pack()
>> +
>> +#endif
>> --
>> 2.17.1
>>
>>
>>
>>
>>
>
> Regards,
> Rohit
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 22933 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-23 20:10 ` Andrew Fish
@ 2022-08-23 21:28 ` Rohit Mathew
2022-08-23 22:59 ` Andrew Fish
2022-08-24 14:39 ` hesham.almatary
1 sibling, 1 reply; 15+ messages in thread
From: Rohit Mathew @ 2022-08-23 21:28 UTC (permalink / raw)
To: Andrew Fish, devel@edk2.groups.io, Swatisri Kantamsetti
Cc: Sami Mujawar, Alexei Fedorov, Mike Kinney,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com, Thomas Abraham,
Thanu Rangarajan, nd
[-- Attachment #1: Type: text/plain, Size: 6457 bytes --]
Hi Andrew,
From: Andrew Fish <afish@apple.com>
Sent: 23 August 2022 21:11
To: devel@edk2.groups.io; Rohit Mathew <Rohit.Mathew@arm.com>
Cc: username@nvidia.com; Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; Mike Kinney <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn; zhiguang.liu@intel.com; Swatisri Kantamsetti <swatisrik@nvidia.com>; Thomas Abraham <thomas.abraham@arm.com>; Thanu Rangarajan <Thanu.Rangarajan@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
On Aug 19, 2022, at 1:26 AM, Rohit Mathew <rohit.mathew@arm.com<mailto:rohit.mathew@arm.com>> wrote:
Hi Swatisri,
Thanks for the patch. Please find my comments inline marked [Rohit] -
-----Original Message-----
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Name
via groups.io<http://groups.io/>
Sent: 16 August 2022 21:18
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>;
Alexei Fedorov <Alexei.Fedorov@arm.com<mailto:Alexei.Fedorov@arm.com>>; michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>;
gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>
Cc: Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>
Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
From: Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>
Added MPAM table header, MSC and Resource Node info structures
Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>
---
MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
MdePkg/Include/IndustryStandard/Mpam.h | 69
++++++++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
b/MdePkg/Include/IndustryStandard/Acpi64.h
index fe5ebfac2b..e54f631186 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2952,6 +2952,11 @@ typedef struct {
///
#define
EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SI
GNATURE SIGNATURE_32('P', 'P', 'T', 'T')
+///
+/// "MPAM" Memory System Resource Partitioning And Monitoring Table
///
+#define
+EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
NG_TABLE_STRUC
+TURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
+
///
/// "PSDT" Persistent System Description Table /// diff --git
a/MdePkg/Include/IndustryStandard/Mpam.h
b/MdePkg/Include/IndustryStandard/Mpam.h
new file mode 100644
index 0000000000..e0f75f0114
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Mpam.h
@@ -0,0 +1,69 @@
+/** @file
+ ACPI Memory System Resource Partitioning And Monitoring (MPAM)
+ as specified in ARM spec DEN0065
+
+ Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2022, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent **/
+
+#ifndef _MPAM_H_
+#define _MPAM_H_
+
+#pragma pack(1)
+
+///
+/// Memory System Resource Partitioning and Monitoring Table (MPAM)
///
+typedef struct {
+ EFI_ACPI_DESCRIPTION_HEADER Header;
+ UINT32 NumNodes;
+ UINT32 NodeOffset;
+ UINT32 Reserved;
+}
+EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
NG_TABLE_HEADE
+R;
[Rohit] Shouldn't the header be followed by MSC node body type as defined in MPAM ACPI 1.0, section 2, table 3 - The MPAM table and section 2.1, table 4 - MSC Node body?
+
+///
+/// MPAM Revision (as defined in ACPI 6.4 spec.) /// #define
+EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
NG_TABLE_REVIS
+ION 0x01
+
+///
+/// Memory System Controller Node Structure ///
+
+typedef struct {
+ UINT16 Length;
+ UINT16 Reserved;
+ UINT32 Identifier;
+ UINT64 BaseAddress;
+ UINT32 MmioSize;
+ UINT32 OverflowInterrupt;
+ UINT32 OverflowInterruptFlags;
[Rohit] Would it be better to have a type (possibly bitfield struct) instead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - Interrupt flags)
Probably better NOT to use bitfields in APIs that are produced and consumed by different worlds. While the the UEFI does speak to the bit order of or a bitfield the rules around packing of bitfields is compiler defined.
I just saw a bug last week with bitfield compatibility that was introduced by clang fixing its -mms-bitfields implementation. The GCC rules for packing bitfields is different than VC++ so that is why the compiler flag -mms-bitfields exists in the 1st place . A clang -mms-bitfields bug got fixed and it broke the code as the extra padding required by VC++ got added to the bitfield.
[Rohit] Thanks for bringing this point. I think, this type could be left untouched in that case.
Thanks,
Andrew Fish
+ UINT32 Reserved1;
+ UINT32 OverflowInterruptAff;
+ UINT32 ErrorInterrupt;
+ UINT32 ErrorInterruptFlags;
[Rohit ] Same comment as before above.
+ UINT32 Reserved2;
+ UINT32 ErrorInterruptAff;
+ UINT32 MaxNRdyUsec;
+ UINT64 LinkedDeviceHwId;
+ UINT32 LinkedDeviceInstanceHwId;
+ UINT32 NumResourceNodes;
+} EFI_ACPI_6_4_MPAM_MSC_NODE;
+
+///
+/// Resource Node Structure
+///
+
+typedef struct {
+ UINT32 Identifier;
+ UINT8 RisIndex;
+ UINT16 Reserved1;
+ UINT8 LocatorType;
+ UINT64 Locator;
[Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
+ UINT32 NumFuncDep;
+} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
[Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NODE type and this could be non-zero, should we also need the type for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Functional dependency descriptor)
[Rohit] Also, could some of the commonly used macros be added to this header, please? (location types, MPAM interrupt mode, interrupt types, affinity type, etc)
+
+#pragma pack()
+
+#endif
--
2.17.1
Regards,
Rohit
[-- Attachment #2: Type: text/html, Size: 16593 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-23 21:28 ` Rohit Mathew
@ 2022-08-23 22:59 ` Andrew Fish
2022-08-24 10:42 ` Rohit Mathew
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Fish @ 2022-08-23 22:59 UTC (permalink / raw)
To: Rohit Mathew
Cc: devel@edk2.groups.io, Swatisri Kantamsetti, Sami Mujawar,
Alexei Fedorov, Mike Kinney, gaoliming@byosoft.com.cn,
zhiguang.liu@intel.com, Thomas Abraham, Thanu Rangarajan, nd
[-- Attachment #1: Type: text/plain, Size: 7678 bytes --]
Rohit,
FYI I seem to remember when we added the bitfield verbiage to the UEFI Spec it was because there was lots of platform code that was using it. We did not really want to encourage its use it in public interfaces.
Given there is lots of code 1st kind of things going on I’d figured I’d mention this.
Thanks,
Andrew Fish
> On Aug 23, 2022, at 2:28 PM, Rohit Mathew <rohit.mathew@arm.com> wrote:
>
> Hi Andrew,
>
> From: Andrew Fish <afish@apple.com <mailto:afish@apple.com>>
> Sent: 23 August 2022 21:11
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Rohit Mathew <Rohit.Mathew@arm.com <mailto:Rohit.Mathew@arm.com>>
> Cc: username@nvidia.com <mailto:username@nvidia.com>; Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>; Alexei Fedorov <Alexei.Fedorov@arm.com <mailto:Alexei.Fedorov@arm.com>>; Mike Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>; Swatisri Kantamsetti <swatisrik@nvidia.com <mailto:swatisrik@nvidia.com>>; Thomas Abraham <thomas.abraham@arm.com <mailto:thomas.abraham@arm.com>>; Thanu Rangarajan <Thanu.Rangarajan@arm.com <mailto:Thanu.Rangarajan@arm.com>>; nd <nd@arm.com <mailto:nd@arm.com>>
> Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
>
>
>
>
> On Aug 19, 2022, at 1:26 AM, Rohit Mathew <rohit.mathew@arm.com <mailto:rohit.mathew@arm.com>> wrote:
>
> Hi Swatisri,
>
> Thanks for the patch. Please find my comments inline marked [Rohit] -
>
>
> -----Original Message-----
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Name
> via groups.io <http://groups.io/>
> Sent: 16 August 2022 21:18
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>;
> Alexei Fedorov <Alexei.Fedorov@arm.com <mailto:Alexei.Fedorov@arm.com>>; michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>;
> gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>
> Cc: Swatisri Kantamsetti <swatisrik@nvidia.com <mailto:swatisrik@nvidia.com>>
> Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
>
> From: Swatisri Kantamsetti <swatisrik@nvidia.com <mailto:swatisrik@nvidia.com>>
>
> Added MPAM table header, MSC and Resource Node info structures
>
> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com <mailto:swatisrik@nvidia.com>>
> ---
> MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
> MdePkg/Include/IndustryStandard/Mpam.h | 69
> ++++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
> create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
>
> diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
> b/MdePkg/Include/IndustryStandard/Acpi64.h
> index fe5ebfac2b..e54f631186 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi64.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi64.h
> @@ -2952,6 +2952,11 @@ typedef struct {
> ///
> #define
> EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SI
> GNATURE SIGNATURE_32('P', 'P', 'T', 'T')
>
> +///
> +/// "MPAM" Memory System Resource Partitioning And Monitoring Table
> ///
> +#define
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_STRUC
> +TURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
> +
> ///
> /// "PSDT" Persistent System Description Table /// diff --git
> a/MdePkg/Include/IndustryStandard/Mpam.h
> b/MdePkg/Include/IndustryStandard/Mpam.h
> new file mode 100644
> index 0000000000..e0f75f0114
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Mpam.h
> @@ -0,0 +1,69 @@
> +/** @file
> + ACPI Memory System Resource Partitioning And Monitoring (MPAM)
> + as specified in ARM spec DEN0065
> +
> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2022, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#ifndef _MPAM_H_
> +#define _MPAM_H_
> +
> +#pragma pack(1)
> +
> +///
> +/// Memory System Resource Partitioning and Monitoring Table (MPAM)
> ///
> +typedef struct {
> + EFI_ACPI_DESCRIPTION_HEADER Header;
> + UINT32 NumNodes;
> + UINT32 NodeOffset;
> + UINT32 Reserved;
> +}
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_HEADE
> +R;
>
> [Rohit] Shouldn't the header be followed by MSC node body type as defined in MPAM ACPI 1.0, section 2, table 3 - The MPAM table and section 2.1, table 4 - MSC Node body?
>
>
> +
> +///
> +/// MPAM Revision (as defined in ACPI 6.4 spec.) /// #define
> +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> NG_TABLE_REVIS
> +ION 0x01
> +
> +///
> +/// Memory System Controller Node Structure ///
> +
> +typedef struct {
> + UINT16 Length;
> + UINT16 Reserved;
> + UINT32 Identifier;
> + UINT64 BaseAddress;
> + UINT32 MmioSize;
> + UINT32 OverflowInterrupt;
> + UINT32 OverflowInterruptFlags;
>
> [Rohit] Would it be better to have a type (possibly bitfield struct) instead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - Interrupt flags)
>
>
> Probably better NOT to use bitfields in APIs that are produced and consumed by different worlds. While the the UEFI does speak to the bit order of or a bitfield the rules around packing of bitfields is compiler defined.
>
> I just saw a bug last week with bitfield compatibility that was introduced by clang fixing its -mms-bitfields implementation. The GCC rules for packing bitfields is different than VC++ so that is why the compiler flag -mms-bitfields exists in the 1st place . A clang -mms-bitfields bug got fixed and it broke the code as the extra padding required by VC++ got added to the bitfield.
>
> [Rohit] Thanks for bringing this point. I think, this type could be left untouched in that case.
>
> Thanks,
>
> Andrew Fish
>
>
> + UINT32 Reserved1;
> + UINT32 OverflowInterruptAff;
> + UINT32 ErrorInterrupt;
> + UINT32 ErrorInterruptFlags;
>
> [Rohit ] Same comment as before above.
>
>
> + UINT32 Reserved2;
> + UINT32 ErrorInterruptAff;
> + UINT32 MaxNRdyUsec;
> + UINT64 LinkedDeviceHwId;
> + UINT32 LinkedDeviceInstanceHwId;
> + UINT32 NumResourceNodes;
> +} EFI_ACPI_6_4_MPAM_MSC_NODE;
> +
> +///
> +/// Resource Node Structure
> +///
> +
> +typedef struct {
> + UINT32 Identifier;
> + UINT8 RisIndex;
> + UINT16 Reserved1;
> + UINT8 LocatorType;
> + UINT64 Locator;
>
> [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
>
>
> + UINT32 NumFuncDep;
> +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
>
> [Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NODE type and this could be non-zero, should we also need the type for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Functional dependency descriptor)
>
> [Rohit] Also, could some of the commonly used macros be added to this header, please? (location types, MPAM interrupt mode, interrupt types, affinity type, etc)
>
>
> +
> +#pragma pack()
> +
> +#endif
> --
> 2.17.1
>
>
>
>
>
>
> Regards,
> Rohit
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 19516 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-23 22:59 ` Andrew Fish
@ 2022-08-24 10:42 ` Rohit Mathew
0 siblings, 0 replies; 15+ messages in thread
From: Rohit Mathew @ 2022-08-24 10:42 UTC (permalink / raw)
To: Andrew Fish
Cc: devel@edk2.groups.io, Swatisri Kantamsetti, Sami Mujawar,
Alexei Fedorov, Mike Kinney, gaoliming@byosoft.com.cn,
zhiguang.liu@intel.com, Thomas Abraham, Thanu Rangarajan, nd
That makes sense, Andrew. Thanks for the info.
Regards,
Rohit
> Rohit,
>
> FYI I seem to remember when we added the bitfield verbiage to the UEFI Spec it was because there was lots of platform code that was using it. We did not really want to encourage its use it in public interfaces.
>
> Given there is lots of code 1st kind of things going on I’d figured I’d mention this.
>
> Thanks,
>
> Andrew Fish
>
>
> On Aug 23, 2022, at 2:28 PM, Rohit Mathew <rohit.mathew@arm.com> wrote:
> >
> > Hi Andrew,
> >
> > From: Andrew Fish <afish@apple.com>
> > Sent: 23 August 2022 21:11
> > To: devel@edk2.groups.io; Rohit Mathew <Rohit.Mathew@arm.com>
> > Cc: username@nvidia.com; Sami Mujawar <Sami.Mujawar@arm.com> ; Alexei Fedorov <Alexei.Fedorov@arm.com> ; Mike Kinney <michael.d.kinney@intel.com> ; gaoliming@byosoft.com.cn; zhiguang.liu@intel.com; Swatisri Kantamsetti <swatisrik@nvidia.com> ; Thomas Abraham <thomas.abraham@arm.com> ; Thanu Rangarajan <Thanu.Rangarajan@arm.com> ; nd <nd@arm.com>
> > Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
> >
> >
> >
> >
> >
> > On Aug 19, 2022, at 1:26 AM, Rohit Mathew <rohit.mathew@arm.com> wrote:
> > > >
> > > > Hi Swatisri,
> > > >
> > > > Thanks for the patch. Please find my comments inline marked [Rohit] -
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Name
> > > > > via groups.io
> > > > > Sent: 16 August 2022 21:18
> > > > > To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com> ;
> > > > > Alexei Fedorov <Alexei.Fedorov@arm.com> ; michael.d.kinney@intel.com;
> > > > > gaoliming@byosoft.com.cn; zhiguang.liu@intel.com
> > > > > Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
> > > > > Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
> > > > >
> > > > > From: Swatisri Kantamsetti <swatisrik@nvidia.com>
> > > > >
> > > > > Added MPAM table header, MSC and Resource Node info structures
> > > > >
> > > > > Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> > > > > ---
> > > > > MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
> > > > > MdePkg/Include/IndustryStandard/Mpam.h | 69
> > > > > ++++++++++++++++++++++++
> > > > > 2 files changed, 74 insertions(+)
> > > > > create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
> > > > >
> > > > > diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
> > > > > b/MdePkg/Include/IndustryStandard/Acpi64.h
> > > > > index fe5ebfac2b..e54f631186 100644
> > > > > --- a/MdePkg/Include/IndustryStandard/Acpi64.h
> > > > > +++ b/MdePkg/Include/IndustryStandard/Acpi64.h
> > > > > @@ -2952,6 +2952,11 @@ typedef struct {
> > > > > ///
> > > > > #define
> > > > > EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SI
> > > > > GNATURE SIGNATURE_32('P', 'P', 'T', 'T')
> > > > >
> > > > > +///
> > > > > +/// "MPAM" Memory System Resource Partitioning And Monitoring Table
> > > > > ///
> > > > > +#define
> > > > > +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> > > > > NG_TABLE_STRUC
> > > > > +TURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
> > > > > +
> > > > > ///
> > > > > /// "PSDT" Persistent System Description Table /// diff --git
> > > > > a/MdePkg/Include/IndustryStandard/Mpam.h
> > > > > b/MdePkg/Include/IndustryStandard/Mpam.h
> > > > > new file mode 100644
> > > > > index 0000000000..e0f75f0114
> > > > > --- /dev/null
> > > > > +++ b/MdePkg/Include/IndustryStandard/Mpam.h
> > > > > @@ -0,0 +1,69 @@
> > > > > +/** @file
> > > > > + ACPI Memory System Resource Partitioning And Monitoring (MPAM)
> > > > > + as specified in ARM spec DEN0065
> > > > > +
> > > > > + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> > > > > + Copyright (c) 2022, ARM Limited. All rights reserved.
> > > > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > > > > +
> > > > > +#ifndef _MPAM_H_
> > > > > +#define _MPAM_H_
> > > > > +
> > > > > +#pragma pack(1)
> > > > > +
> > > > > +///
> > > > > +/// Memory System Resource Partitioning and Monitoring Table (MPAM)
> > > > > ///
> > > > > +typedef struct {
> > > > > + EFI_ACPI_DESCRIPTION_HEADER Header;
> > > > > + UINT32 NumNodes;
> > > > > + UINT32 NodeOffset;
> > > > > + UINT32 Reserved;
> > > > > +}
> > > > > +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> > > > > NG_TABLE_HEADE
> > > > > +R;
> > > >
> > > > [Rohit] Shouldn't the header be followed by MSC node body type as defined in MPAM ACPI 1.0, section 2, table 3 - The MPAM table and section 2.1, table 4 - MSC Node body?
> > > >
> > > >
> > > >
> > > > > +
> > > > > +///
> > > > > +/// MPAM Revision (as defined in ACPI 6.4 spec.) /// #define
> > > > > +EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
> > > > > NG_TABLE_REVIS
> > > > > +ION 0x01
> > > > > +
> > > > > +///
> > > > > +/// Memory System Controller Node Structure ///
> > > > > +
> > > > > +typedef struct {
> > > > > + UINT16 Length;
> > > > > + UINT16 Reserved;
> > > > > + UINT32 Identifier;
> > > > > + UINT64 BaseAddress;
> > > > > + UINT32 MmioSize;
> > > > > + UINT32 OverflowInterrupt;
> > > > > + UINT32 OverflowInterruptFlags;
> > > >
> > > > [Rohit] Would it be better to have a type (possibly bitfield struct) instead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - Interrupt flags)
> > > >
> > > >
> > > >
> > > Probably better NOT to use bitfields in APIs that are produced and consumed by different worlds. While the the UEFI does speak to the bit order of or a bitfield the rules around packing of bitfields is compiler defined.
> > >
> > > I just saw a bug last week with bitfield compatibility that was introduced by clang fixing its -mms-bitfields implementation. The GCC rules for packing bitfields is different than VC++ so that is why the compiler flag -mms-bitfields exists in the 1st place . A clang -mms-bitfields bug got fixed and it broke the code as the extra padding required by VC++ got added to the bitfield.
> >
> > [Rohit] Thanks for bringing this point. I think, this type could be left untouched in that case.
> >
> > > Thanks,
> > >
> > > Andrew Fish
> > >
> > >
> > >
> > > > > + UINT32 Reserved1;
> > > > > + UINT32 OverflowInterruptAff;
> > > > > + UINT32 ErrorInterrupt;
> > > > > + UINT32 ErrorInterruptFlags;
> > > >
> > > > [Rohit ] Same comment as before above.
> > > >
> > > >
> > > >
> > > > > + UINT32 Reserved2;
> > > > > + UINT32 ErrorInterruptAff;
> > > > > + UINT32 MaxNRdyUsec;
> > > > > + UINT64 LinkedDeviceHwId;
> > > > > + UINT32 LinkedDeviceInstanceHwId;
> > > > > + UINT32 NumResourceNodes;
> > > > > +} EFI_ACPI_6_4_MPAM_MSC_NODE;
> > > > > +
> > > > > +///
> > > > > +/// Resource Node Structure
> > > > > +///
> > > > > +
> > > > > +typedef struct {
> > > > > + UINT32 Identifier;
> > > > > + UINT8 RisIndex;
> > > > > + UINT16 Reserved1;
> > > > > + UINT8 LocatorType;
> > > > > + UINT64 Locator;
> > > >
> > > > [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
> > > >
> > > >
> > > >
> > > > > + UINT32 NumFuncDep;
> > > > > +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
> > > >
> > > > [Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NODE type and this could be non-zero, should we also need the type for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Functional dependency descriptor)
> > > >
> > > > [Rohit] Also, could some of the commonly used macros be added to this header, please? (location types, MPAM interrupt mode, interrupt types, affinity type, etc)
> > > >
> > > >
> > > >
> > > > > +
> > > > > +#pragma pack()
> > > > > +
> > > > > +#endif
> > > > > --
> > > > > 2.17.1
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > Regards,
> > > > Rohit
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files
2022-08-19 8:26 ` [edk2-devel] " Rohit Mathew
@ 2022-08-24 10:51 ` Rohit Mathew
0 siblings, 0 replies; 15+ messages in thread
From: Rohit Mathew @ 2022-08-24 10:51 UTC (permalink / raw)
To: devel@edk2.groups.io, Swatisri Kantamsetti
Cc: Thomas Abraham, Thanu Rangarajan, nd, Andrew Fish, Sami Mujawar,
Alexei Fedorov, michael.d.kinney@intel.com,
gaoliming@byosoft.com.cn, zhiguang.liu@intel.com
Hi Swatisri,
I have dropped comments on the Flag's type based on Andrew's feedback.
Regards,
Rohit
> -----Original Message-----
> From: Rohit Mathew
> Sent: 19 August 2022 09:26
> To: devel@edk2.groups.io; username@nvidia.com; Sami Mujawar
> <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>;
> michael.d.kinney@intel.com; gaoliming@byosoft.com.cn;
> zhiguang.liu@intel.com
> Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>; Thomas Abraham
> <thomas.abraham@arm.com>; Thanu Rangarajan
> <Thanu.Rangarajan@arm.com>; nd <nd@arm.com>
> Subject: RE: [edk2-devel] [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM
> Generator and supporting files
>
> Hi Swatisri,
>
> Thanks for the patch. Please find my comments inline marked [Rohit] -
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Name
> > via groups.io
> > Sent: 16 August 2022 21:18
> > To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>;
> Alexei
> > Fedorov <Alexei.Fedorov@arm.com>; michael.d.kinney@intel.com;
> > gaoliming@byosoft.com.cn; zhiguang.liu@intel.com
> > Cc: Swatisri Kantamsetti <swatisrik@nvidia.com>
> > Subject: [edk2-devel] [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM
> > Generator and supporting files
> >
> > From: Swatisri Kantamsetti <swatisrik@nvidia.com>
> >
> > ACPI header, MSC and Resource Nodes are populated in the MPAM Table
> >
> > Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> > ---
> > DynamicTablesPkg/DynamicTables.dsc.inc | 2 +
> > DynamicTablesPkg/Include/AcpiTableGenerator.h | 1 +
> > .../Include/ArmNameSpaceObjects.h | 68 ++
> > .../Arm/AcpiMpamLibArm/AcpiMpamLibArm.inf | 30 +
> > .../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c | 649
> > ++++++++++++++++++
> > .../Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h | 47 ++
> > 6 files changed, 797 insertions(+)
> > create mode 100644
> >
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.in
> > f
> > create mode 100644
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.c
> > create mode 100644
> >
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.h
> >
> > diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc
> > b/DynamicTablesPkg/DynamicTables.dsc.inc
> > index 3d4fa0c4c4..745d5f0633 100644
> > --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> > +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> > @@ -29,6 +29,7 @@
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
> >
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
> >
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
> > +
> >
> DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm.in
> > f
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.inf
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
> > @@ -54,6 +55,7 @@
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibAr
> > m.inf
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibAr
> > m.inf
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.
> > inf
> > +
> > +
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLib
> > Arm.i
> > + nf
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/AcpiRawLibArm.
> > inf
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.
> > inf
> >
> >
> NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/AcpiSratLibArm.i
> > nf
> > diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h
> > b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> > index f962dbff57..56d7375b4a 100644
> > --- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
> > +++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
> > @@ -94,6 +94,7 @@ typedef enum StdAcpiTableId {
> > EStdAcpiTableIdIort, ///< IORT Generator
> > EStdAcpiTableIdPptt, ///< PPTT Generator
> > EStdAcpiTableIdSrat, ///< SRAT Generator
> > + EStdAcpiTableIdMpam, ///< MPAM Generator
> > EStdAcpiTableIdSsdtSerialPort, ///< SSDT Serial-Port Generator
> > EStdAcpiTableIdSsdtCmn600, ///< SSDT Cmn-600 Generator
> > EStdAcpiTableIdSsdtCpuTopology, ///< SSDT Cpu Topology
> > diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> > b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> > index 102e0f96be..39a14ed0b3 100644
> > --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> > +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> > @@ -63,6 +63,8 @@ typedef enum ArmObjectID {
> > EArmObjPciInterruptMapInfo, ///< 39 - Pci Interrupt Map Info
> > EArmObjRmr, ///< 40 - Reserved Memory Range Node
> > EArmObjMemoryRangeDescriptor, ///< 41 - Memory Range
> Descriptor
> > + EArmObjMscNodeInfo, ///< 40 - Msc Memory System Controller
> > Node Info
> > + EArmObjResNodeInfo, ///< 41 - Res Resource Node Info
> > EArmObjMax
> > } EARM_OBJECT_ID;
> >
> > @@ -1070,6 +1072,72 @@ typedef struct CmArmRmrDescriptor {
> > UINT64 Length;
> > } CM_ARM_MEMORY_RANGE_DESCRIPTOR;
> >
> > +/** A structure that describes Memory System Controller Node.
> > +
> > + MPAM Memory System Component Nodes are described by
> > + this object.
> > +
> > + ID: EArmObjMscNodeInfo
> > +*/
> > +typedef struct CmArmMscNodeInfo {
> > + /// An unique token used to identify this object
> > + CM_OBJECT_TOKEN Token;
> > +
> > + /// MPAM Base Address
> > + UINT64 BaseAddress;
> > + /// MMIO Size
> > + UINT32 MmioSize;
> > + /// Overflow Interrupt
> > + UINT32 OverflowInterrupt;
> > + /// Overflow Interrupt Flags
> > + UINT32 OverflowInterruptFlags;
>
> [Rohit] I have added a comment on Flag's type in [PATCH 1/2]. Same
> comment here.
[Rohit] Please ignore this.
>
> > + /// Overflow Interrupt Affinity
> > + UINT32 OverflowInterruptAff;
> > + /// Error Interrupt
> > + UINT32 ErrorInterrupt;
> > + /// Error Interrupt Flags
> > + UINT32 ErrorInterruptFlags;
> > + /// Error Interrupt Affinity
> > + UINT32 ErrorInterruptAff;
> > + /// Not Ready Signal time
> > + UINT32 MaxNRdyUsec;
> > + /// Linked Device HWID
> > + UINT64 LinkedDeviceHwId;
> > + /// Linked Device Instance ID
> > + UINT32 LinkedDeviceInstanceHwId;
> > + /// Number of Resource nodes
> > + UINT32 NumResourceNodes;
> > + /// Reference token for the list of resource nodes
> > + //CM_OBJECT_TOKEN ResourceNodeListToken;
> > +
> > +} CM_ARM_MSC_NODE_INFO;
> > +
> > +/** A structure that describes Memory System Controller Node.
> > +
> > + MPAM Memory System Component Nodes are described by
> > + this object.
> > +
> > + ID: EArmObjResNodeInfo
> > +*/
> > +typedef struct CmArmResNodeInfo {
> > + /// An unique token used to identify this object
> > + CM_OBJECT_TOKEN Token;
> > +
> > + /// Identifier
> > + UINT32 Identifier;
> > + /// RIS Index
> > + UINT8 RisIndex;
> > + /// Locator Type
> > + UINT8 LocatorType;
> > + /// Locator
> > + UINT64 Locator;
>
> [Rohit] I have added a comment on Locator's type and size in [PATCH 1/2].
> Same comment here.
>
> > + /// Num functional dependencies
> > + UINT32 NumFuncDep;
> > + /// Reference token for the list of resource nodes
> > + CM_OBJECT_TOKEN FuncDepListToken;
> > +
> > +} CM_ARM_RESOURCE_NODE_INFO;
> > +
> > #pragma pack()
> >
> > #endif // ARM_NAMESPACE_OBJECTS_H_
> > diff --git
> >
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> > .inf
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> > .inf
> > new file mode 100644
> > index 0000000000..480130dc21
> > --- /dev/null
> > +++
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/AcpiMpamLibArm
> > .in
> > +++ f
> > @@ -0,0 +1,30 @@
> > +## @file
> > +# MPAM Table Generator Inf file
> > +#
> > +# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> > +# Copyright (c) 2022, ARM Limited. All rights reserved.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> > +
> > +[Defines]
> > + INF_VERSION = 0x0001001B
> > + BASE_NAME = AcpiMpamLibArm
> > + FILE_GUID = 02d0c79f-41cd-45c9-9835-781229c619d1
> > + VERSION_STRING = 1.0
> > + MODULE_TYPE = DXE_DRIVER
> > + LIBRARY_CLASS = NULL|DXE_DRIVER
> > + CONSTRUCTOR = AcpiMpamLibConstructor
> > + DESTRUCTOR = AcpiMpamLibDestructor
> > +
> > +[Sources]
> > + MpamGenerator.c
> > + MpamGenerator.h
> > +
> > +[Packages]
> > + EmbeddedPkg/EmbeddedPkg.dec
> > + DynamicTablesPkg/DynamicTablesPkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > + MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > + BaseLib
> > diff --git
> >
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > c
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > c
> > new file mode 100644
> > index 0000000000..db3e8e95bc
> > --- /dev/null
> > +++
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > c
> > @@ -0,0 +1,649 @@
> > +/** @file
> > + MPAM Table Generator
> > +
> > + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2022, ARM Limited. All rights reserved.
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > + @par Reference(s):
> > + - ACPI 6.4 Specification, January 2021
> > +
> > + @par Glossary:
> > + - Cm or CM - Configuration Manager
> > + - Obj or OBJ - Object
> > +**/
> > +
> > +#include <IndustryStandard/Mpam.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>
> > +
> > +#include "MpamGenerator.h"
> > +
> > +/**
> > + ARM standard MPAM Generator
> > +
> > + Requirements:
> > + The following Configuration Manager Object(s) are used by this
> > Generator:
> > + - EArmObjMscNodeInfo (REQUIRED)
> > + - EArmObjResNodeInfo
> > +*/
> > +
> > +/**
> > + This macro expands to a function that retrieves the MSC Node
> > +information
> > + from the Configuration Manager.
> > +*/
> > +GET_OBJECT_LIST (
> > + EObjNameSpaceArm,
> > + EArmObjMscNodeInfo,
> > + CM_ARM_MSC_NODE_INFO
> > + );
> > +
> > +/**
> > + This macro expands to a function that retrieves the Resource Node
> > + information from the Configuration Manager.
> > +*/
> > +GET_OBJECT_LIST (
> > + EObjNameSpaceArm,
> > + EArmObjResNodeInfo,
> > + CM_ARM_RESOURCE_NODE_INFO
> > + );
> > +
> > +/**
> > + Returns the size of the MPAM Memory System Controller (MSC) Node
> > +
> > + @param [in] Node Pointer to MSC Node Info CM object
> > +
> > + @retval Size of the MSC Node in bytes.
> > +**/
> > +STATIC
> > +UINT32
> > +GetMscNodeSize (
> > + IN CONST CM_ARM_MSC_NODE_INFO *Node
> > + )
> > +{
> > + ASSERT (Node != NULL);
> > +
> > + // <size of Memory System Controller Node>
> > + return (UINT32)(sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE) +
> > + Node->NumResourceNodes * sizeof
> > +(EFI_ACPI_6_4_MPAM_RESOURCE_NODE));
>
> [Rohit] Shouldn't the node size include the size of functional dependency
> descriptor * number of functional dependency descriptor?
>
> > +}
> > +
> > +/** Returns the total size required for the MSC and
> > + updates the Node Indexer.
> > +
> > + This function calculates the size required for the node group
> > + and also populates the Node Indexer array with offsets for the
> > + individual nodes.
> > +
> > + @param [in] NodeStartOffset Offset from the start of the
> > + MPAM where this node group starts.
>
> [Rohit] Should it be "start of the MPAM table where .."?
>
> > + @param [in] NodeList Pointer to MSC Group node list.
> > + @param [in] NodeCount Count of the MSC Group nodes.
> > + @param [in, out] NodeIndexer Pointer to the next Node Indexer.
> > +
> > + @retval Total size of the ITS Group Nodes.
> > +**/
> > +STATIC
> > +UINT64
> > +GetSizeofMscGroupNodes (
> > + IN CONST UINT32 NodeStartOffset,
> > + IN CONST CM_ARM_MSC_NODE_INFO *NodeList,
> > + IN UINT32 NodeCount,
> > + IN OUT MPAM_NODE_INDEXER **CONST NodeIndexer
> > + )
> > +{
> > + UINT64 Size;
> > +
> > + ASSERT (NodeList != NULL);
> > +
> > + Size = 0;
> > + while (NodeCount-- != 0) {
> > + (*NodeIndexer)->Token = NodeList->Token;
> > + (*NodeIndexer)->Object = (VOID *)NodeList;
> > + (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "MPAM: MSC Node Indexer = %p, Token = %p, Object = %p, Offset =
> > 0x%x\n",
> > + *NodeIndexer,
> > + (*NodeIndexer)->Token,
> > + (*NodeIndexer)->Object,
> > + (*NodeIndexer)->Offset
> > + ));
> > +
> > + Size += GetMscNodeSize (NodeList);
> > +
> > + (*NodeIndexer)++;
> > + NodeList++;
> > + }
> > +
> > + return Size;
> > +}
> > +
> > +/** Update the MSC Group Node Information.
> > +
> > + @param [in] This Pointer to the table Generator.
> > + @param [in] CfgMgrProtocol Pointer to the Configuration Manager
> > + Protocol Interface.
> > + @param [in] Mpam Pointer to MPAM table structure.
> > + @param [in] NodesStartOffset Offset for the start of the Msc Group
> > + Nodes.
> > + @param [in] NodeList Pointer to an array of Msc Group Node
> > + Objects.
> > + @param [in] NodeCount Number of Msc Group Node Objects.
> > +
> > + @retval EFI_SUCCESS Table generated successfully.
> > + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> > + @retval EFI_NOT_FOUND The required object was not found.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +AddMscNodes (
> > + IN CONST ACPI_TABLE_GENERATOR
> *CONST
> > This,
> > + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL
> > *CONST CfgMgrProtocol,
> > + IN CONST
> >
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_HEADER *Mpam,
> > + IN CONST UINT32
> > NodesStartOffset,
> > + IN CONST CM_ARM_MSC_NODE_INFO
> > *NodeList,
> > + IN UINT32 NodeCount
> > + )
> > +{
> > + EFI_STATUS Status;
> > + EFI_ACPI_6_4_MPAM_MSC_NODE *MscNode;
> > + EFI_ACPI_6_4_MPAM_RESOURCE_NODE *ResourceNodeArray;
> > + CM_ARM_RESOURCE_NODE_INFO *ResourceNodeList;
> > + UINT64 NodeLength;
> > + UINT32 ResourceNodeCount;
> > +
> > + ASSERT (Mpam != NULL);
> > +
> > + MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)Mpam +
> > + NodesStartOffset);
> > +
> > + // Get the Resource Node info to update the MPAM MSC node Status =
> > + GetEArmObjResNodeInfo (
> > + CfgMgrProtocol,
> > + CM_NULL_TOKEN,
> > + &ResourceNodeList,
> > + &ResourceNodeCount
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to add resource nodes info. Status = %r\n",
> > + Status
> > + ));
> > + return Status;
> > + }
> > +
> > + while (NodeCount-- != 0) {
> > + NodeLength = GetMscNodeSize (NodeList);
> > +
> > + if (NodeLength > MAX_UINT16) {
> > + Status = EFI_INVALID_PARAMETER;
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: MSC Node length 0x%lx > MAX_UINT16. Status =
> > %r\n",
> > + NodeLength,
> > + Status
> > + ));
> > + return Status;
> > + }
> > +
> > + // Populate the node header
> > + MscNode->Length = (UINT16)NodeLength;
> > + MscNode->Reserved = EFI_ACPI_RESERVED_WORD;
> > + MscNode->Identifier = EFI_ACPI_RESERVED_DWORD;
>
> [Rohit] MPAM ACPI 1.0, section 2.1, table 4 - MSC node body, defines
> identifier as a unique value. For systems with multiple MSCs, the OS could
> expect identifiers to be unique. Could this be addressed, please?
>
> > + MscNode->BaseAddress = NodeList->BaseAddress;
> > + MscNode->MmioSize = NodeList->MmioSize;
> > + MscNode->OverflowInterrupt = NodeList->OverflowInterrupt;
> > + MscNode->OverflowInterruptFlags = NodeList-
> >OverflowInterruptFlags;
> > + MscNode->Reserved1 = EFI_ACPI_RESERVED_DWORD;
> > + MscNode->OverflowInterruptAff = NodeList->OverflowInterruptAff;
> > + MscNode->ErrorInterrupt = NodeList->ErrorInterrupt;
> > + MscNode->ErrorInterruptFlags = NodeList->ErrorInterruptFlags;
> > + MscNode->Reserved2 = EFI_ACPI_RESERVED_DWORD;
> > + MscNode->ErrorInterruptAff = NodeList->ErrorInterruptAff;
> > + MscNode->MaxNRdyUsec = NodeList->MaxNRdyUsec;
> > + MscNode->LinkedDeviceHwId = NodeList->LinkedDeviceHwId;
> > + MscNode->LinkedDeviceInstanceHwId = NodeList-
> > >LinkedDeviceInstanceHwId;
> > + MscNode->NumResourceNodes = NodeList->NumResourceNodes;
> > +
> > + // ResourceNode List for each MSC
> > + if (MscNode->NumResourceNodes > 0) {
> > + // Resource Node array for this Msc node
> > + ResourceNodeArray = (EFI_ACPI_6_4_MPAM_RESOURCE_NODE
> > *)((UINT8 *)MscNode + sizeof (EFI_ACPI_6_4_MPAM_MSC_NODE));
> > + // Adding Resource Node content
> > + while ( MscNode->NumResourceNodes-- != 0 ) {
> > + ResourceNodeArray->Identifier = ResourceNodeList->Identifier;
> > + ResourceNodeArray->RisIndex = ResourceNodeList->RisIndex;
> > + ResourceNodeArray->Reserved1 = EFI_ACPI_RESERVED_WORD;
> > + ResourceNodeArray->LocatorType = ResourceNodeList-
> >LocatorType;
> > + ResourceNodeArray->Locator = ResourceNodeList->Locator;
> > + ResourceNodeArray->NumFuncDep = ResourceNodeList-
> > >NumFuncDep;
>
> [Rohit] If NumFuncDep > 0, shouldn't we need an additional loop to populate
> functional dependency list?
>
> > +
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to add Resource Node List. Status = %r\n",
> > + Status
> > + ));
> > + return Status;
> > + }
> > +
> > + ResourceNodeList++;
> > + ResourceNodeArray++;
> > + }
> > + }
> > +
> > + // Next MSC Node
> > + MscNode = (EFI_ACPI_6_4_MPAM_MSC_NODE *)((UINT8 *)MscNode
> +
> > MscNode->Length);
> > + NodeList++;
> > + } // Msc Node
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + Construct the MPAM ACPI table.
> > +
> > + 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 generator to be
> > used.
> > + @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.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +BuildMpamTable (
> > + 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;
> > + UINT64 TableSize;
> > + UINT64 NodeSize;
> > + UINT32 MpamNodeCount;
> > +
> > + UINT32 MscNodeCount;
> > + UINT32 MscNodeOffset;
> > +
> > +
> >
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_HEADER *Mpam;
> > + ACPI_MPAM_GENERATOR *Generator;
> > + CM_ARM_MSC_NODE_INFO *MscNodeList;
> > + MPAM_NODE_INDEXER *NodeIndexer;
> > +
> > + ASSERT (
> > + (This != NULL) &&
> > + (AcpiTableInfo != NULL) &&
> > + (CfgMgrProtocol != NULL) &&
> > + (Table != NULL) &&
> > + (AcpiTableInfo->TableGeneratorId == This->GeneratorID) &&
> > + (AcpiTableInfo->AcpiTableSignature == This->AcpiTableSignature)
> > + );
> > +
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "DEBUG PRINT: MPAM: Requested table revision = %d\n",
> > + AcpiTableInfo->AcpiTableRevision
> > + ));
> > +
> > + if ((AcpiTableInfo->AcpiTableRevision < This->MinAcpiTableRevision) ||
> > + (AcpiTableInfo->AcpiTableRevision > This->AcpiTableRevision)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: 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;
> > + }
> > +
> > + Generator = (ACPI_MPAM_GENERATOR *)This;
> > + *Table = NULL;
> > +
> > + // Get the Memory System Controller Node info and update the MPAM
> > + // structure count with MSC Node count (Type 0) Status =
> > + GetEArmObjMscNodeInfo (
> > + CfgMgrProtocol,
> > + CM_NULL_TOKEN,
> > + &MscNodeList,
> > + &MscNodeCount
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to get memory system controller node info.
> > Status = %r\n",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + MpamNodeCount = MscNodeCount;
> > + Generator->MscNodeCount = MscNodeCount;
> > +
> > + // Allocate Node Indexer array
> > + NodeIndexer = (MPAM_NODE_INDEXER *)AllocateZeroPool (
> > + sizeof (MPAM_NODE_INDEXER) *
> > + MpamNodeCount
> > + ); if (NodeIndexer == NULL) {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to allocate memory for Node Indexer.
> > + Status =
> > %r\n ",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + DEBUG ((DEBUG_INFO, "MPAM INFO: NodeIndexer = %p\n",
> > NodeIndexer));
> > + Generator->MpamNodeCount = MpamNodeCount;
> > + Generator->NodeIndexer = NodeIndexer;
> > +
> > + // Calculate the size of the MPAM table TableSize = sizeof
> > +
> >
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_HEA
> > + DER);
> > +
> > + // Include the size of MSC Nodes and index them if
> > + (Generator->MscNodeCount != 0) {
> > + MscNodeOffset = TableSize;
> > + // Size of MSC nodes.
> > + NodeSize = GetSizeofMscGroupNodes (
> > + MscNodeOffset,
> > + MscNodeList,
> > + Generator->MscNodeCount,
> > + &NodeIndexer
> > + );
>
> [Rohit] MscNodeOffset is sizeof
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> G_TABLE_HEADER), which moves past EFI_ACPI_DESCRIPTION_HEADER by
> 12 bytes. Should we be having these fields in the table type?
> UINT32 NumNodes;
> UINT32 NodeOffset;
> UINT32 Reserved;
>
> I had added a similar comment on [PATCH 1/2].
> [/Rohit]
>
> > + if (NodeSize > MAX_UINT32) {
> > + Status = EFI_INVALID_PARAMETER;
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Invalid Size of Group Nodes. Status = %r\n",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + TableSize += NodeSize;
> > +
> > + DEBUG ((
> > + DEBUG_INFO,
> > + " MscNodeCount = %d\n" \
> > + " MscNodeOffset = 0x%x\n",
> > + Generator->MscNodeCount,
> > + MscNodeOffset
> > + ));
> > + }
> > +
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "INFO: MPAM:\n" \
> > + " MpamNodeCount = %d\n" \
> > + " TableSize = 0x%X\n",
> > + MpamNodeCount,
> > + TableSize
> > + ));
> > +
> > + if (TableSize > MAX_UINT32) {
> > + Status = EFI_INVALID_PARAMETER;
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: MPAM Table Size 0x%lx > MAX_UINT32," \
> > + " Status = %r\n",
> > + TableSize,
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + // Allocate the Buffer for the MPAM table *Table =
> > + (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize); if
> > + (*Table == NULL) {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to allocate memory for MPAM Table. " \
> > + "Size = %d. Status = %r\n",
> > + TableSize,
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + Mpam =
> > +
> >
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_HEA
> > + DER *)*Table;
> > +
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "MPAM: Mpam = 0x%p. TableSize = 0x%x\n",
> > + Mpam,
> > + TableSize
> > + ));
> > +
> > + // Add ACPI header
> > + Status = AddAcpiHeader (
> > + CfgMgrProtocol,
> > + This,
> > + &Mpam->Header,
> > + AcpiTableInfo,
> > + TableSize
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to add ACPI header. Status = %r\n",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > +
> > + // Update MPAM table
> > + Mpam->NumNodes = MscNodeCount;
> > + Mpam->NodeOffset = sizeof
> >
> (EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_HEADER);
> > + Mpam->Reserved = EFI_ACPI_RESERVED_DWORD;
> > +
> > + // Add MSC Nodes to the generated table if (Mpam->NumNodes != 0) {
> > + Status = AddMscNodes (
> > + This,
> > + CfgMgrProtocol,
> > + Mpam,
> > + MscNodeOffset,
> > + MscNodeList,
> > + MscNodeCount
> > + );
>
> [Rohit] Same comment as above
>
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: MPAM: Failed to add MSC Nodes. Status = %r\n",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > + }
> > +
> > + return Status;
> > +
> > +error_handler:
> > + if (Generator->NodeIndexer != NULL) {
> > + FreePool (Generator->NodeIndexer);
> > + Generator->NodeIndexer = NULL;
> > + }
> > +
> > + if (*Table != NULL) {
> > + FreePool (*Table);
> > + *Table = NULL;
> > + }
> > +
> > + return Status;
> > +}
> > +
> > +/** Free any resources allocated for constructing the MPAM
>
> [Rohit] Should this be "constructing the MPAM table"?
>
> > +
> > + @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
> > +FreeMpamTableResources (
> > + 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
> > + )
> > +{
> > + ACPI_MPAM_GENERATOR *Generator;
> > +
> > + ASSERT (This != NULL);
> > + ASSERT (AcpiTableInfo != NULL);
> > + ASSERT (CfgMgrProtocol != NULL);
> > + ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
> > + ASSERT (AcpiTableInfo->AcpiTableSignature ==
> > + This->AcpiTableSignature);
> > +
> > + Generator = (ACPI_MPAM_GENERATOR *)This;
> > +
> > + // Free any memory allocated by the generator if
> > + (Generator->NodeIndexer != NULL) {
> > + FreePool (Generator->NodeIndexer);
> > + Generator->NodeIndexer = NULL;
> > + }
> > +
> > + if ((Table == NULL) || (*Table == NULL)) {
> > + DEBUG ((DEBUG_ERROR, "ERROR: MPAM: Invalid Table Pointer\n"));
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + FreePool (*Table);
> > + *Table = NULL;
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/** The MPAM Table Generator revision.
> > +*/
> > +#define MPAM_GENERATOR_REVISION CREATE_REVISION (1, 0)
> > +
> > +/** The interface for the MPAM Table Generator.
> > +*/
> > +STATIC
> > +ACPI_MPAM_GENERATOR MpamGenerator = {
> > + // ACPI table generator header
> > + {
> > + // Generator ID
> > + CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMpam),
> > + // Generator Description
> > + L"ACPI.STD.MPAM.GENERATOR",
> > + // ACPI Table Signature
> > +
> >
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_STRUCTURE_SIGNATURE,
> > + // ACPI Table Revision supported by this Generator
> > +
> >
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_REVISION,
> > + // Minimum supported ACPI Table Revision
> > +
> >
> EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORIN
> > G_TABLE_REVISION,
> > + // Creator ID
> > + TABLE_GENERATOR_CREATOR_ID_ARM,
> > + // Creator Revision
> > + MPAM_GENERATOR_REVISION,
> > + // Build Table function
> > + BuildMpamTable,
> > + // Free Resource function
> > + FreeMpamTableResources,
> > + // Extended build function not needed
> > + NULL,
> > + // Extended build function not implemented by the generator.
> > + // Hence extended free resource function is not required.
> > + NULL
> > + },
> > +
> > + // MPAM Generator private data
> > +
> > + // MPAM node count
> > + 0,
> > + // MSC node count
> > + 0,
> > +
> > + // Pointer to MPAM Node Indexer
> > + 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
> > +AcpiMpamLibConstructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + EFI_STATUS Status;
> > +
> > + Status = RegisterAcpiTableGenerator (&MpamGenerator.Header);
> > + DEBUG ((DEBUG_INFO, "MPAM: 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
> > +AcpiMpamLibDestructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + EFI_STATUS Status;
> > +
> > + Status = DeregisterAcpiTableGenerator (&MpamGenerator.Header);
> > + DEBUG ((DEBUG_INFO, "MPAM: Deregister Generator. Status = %r\n",
> > +Status));
> > + ASSERT_EFI_ERROR (Status);
> > + return Status;
> > +}
> > diff --git
> >
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > h
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > h
> > new file mode 100644
> > index 0000000000..1075bd8c6c
> > --- /dev/null
> > +++
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMpamLibArm/MpamGenerator.
> > h
> > @@ -0,0 +1,47 @@
> > +/** @file
> > + Header file for the dynamic MPAM generator
> > +
> > + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2022, ARM Limited. All rights reserved.
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > + @par Reference(s):
> > + - ACPI 6.4 Specification, January 2021
> > + - ARM Architecture Reference Manual ARMv8
> > +
> > + @par Glossary:
> > + - Cm or CM - Configuration Manager
> > + - Obj or OBJ - Object
> > +**/
> > +
> > +#ifndef MPAM_GENERATOR_H_
> > +#define MPAM_GENERATOR_H_
> > +
> > +#pragma pack(1)
> > +
> > +/** A structure that describes the Node indexer
> > + used for indexing the MPAM MSC nodes.
> > +*/
> > +typedef struct MpamNodeIndexer {
> > + /// Index token for the Node
> > + CM_OBJECT_TOKEN Token;
> > + /// Pointer to the node
> > + VOID *Object;
> > + /// Node offset from the start of the MPAM table
> > + UINT32 Offset;
> > +} MPAM_NODE_INDEXER;
> > +
> > +typedef struct AcpiMpamGenerator {
> > + /// ACPI Table generator header
> > + ACPI_TABLE_GENERATOR Header;
> > + /// MPAM structure count
> > + UINT32 MpamNodeCount;
> > + /// Count of Msc Nodes
> > + UINT32 MscNodeCount;
> > + /// List of indexed CM objects for MPAM generation
> > + MPAM_NODE_INDEXER *NodeIndexer;
> > +} ACPI_MPAM_GENERATOR;
> > +
> > +#pragma pack()
> > +
> > +#endif // MPAM_GENERATOR_H_
> > --
> > 2.17.1
> >
> >
> >
> >
> >
>
> Regards,
> Rohit
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-23 20:10 ` Andrew Fish
2022-08-23 21:28 ` Rohit Mathew
@ 2022-08-24 14:39 ` hesham.almatary
2022-08-24 16:25 ` Rohit Mathew
1 sibling, 1 reply; 15+ messages in thread
From: hesham.almatary @ 2022-08-24 14:39 UTC (permalink / raw)
To: Andrew Fish, devel
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
Hello Rohit and Swatisri,
On Tue, Aug 23, 2022 at 09:11 PM, Andrew Fish wrote:
>
> [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a
> separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and
> section 2.3.2 table 10 - locator descriptor)
I'd just go for UINT64 locator1 and UINT32 locator2 and not necessarily a separate type. The interpretation of each depends on the the locator type (e.g., MPAM ACPI Tables 11-15).
[-- Attachment #2: Type: text/html, Size: 715 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-24 14:39 ` hesham.almatary
@ 2022-08-24 16:25 ` Rohit Mathew
2022-08-25 9:36 ` hesham.almatary
0 siblings, 1 reply; 15+ messages in thread
From: Rohit Mathew @ 2022-08-24 16:25 UTC (permalink / raw)
To: devel@edk2.groups.io, hesham.almatary@huawei.com,
Swatisri Kantamsetti
Cc: Sami Mujawar, Thomas Abraham, Thanu Rangarajan, nd
[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]
Hi Hesham,
The idea for keeping it as separate type was to abstract locator field to have a view similar to that of its definition in the spec (MPAM ACPI 1.0, section 2.2, table 7 - Resource node.)
Something of the lines –
typedef struct {
UINT32 Identifier;
UINT8 RisIndex;
UINT16 Reserved1;
UINT8 LocatorType;
MPAM_LOCATOR Locator;
UINT32 NumDependencies;
} MPAM_MSC_RESOURCE;
Locator field could very well be a struct of UINT64 and UINT32 with just descriptor 1 and descriptor 2 as the field names (Table 10, 2.3.2)– As you said, having hardcoded field names won’t work as the descriptors change with locator type.
///
/// MPAM MSC Locator field
///
typedef struct {
UINT64 Descriptor1;
UINT32 Descriptor2;
} MPAM_LOCATOR;
Let me know your thoughts on the approach.
Regards,
Rohit
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of hesham.almatary via groups.io
Sent: 24 August 2022 15:39
To: Andrew Fish <afish@apple.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
Hello Rohit and Swatisri,
On Tue, Aug 23, 2022 at 09:11 PM, Andrew Fish wrote:
[Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
I'd just go for UINT64 locator1 and UINT32 locator2 and not necessarily a separate type. The interpretation of each depends on the the locator type (e.g., MPAM ACPI Tables 11-15).
[-- Attachment #2: Type: text/html, Size: 7619 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-24 16:25 ` Rohit Mathew
@ 2022-08-25 9:36 ` hesham.almatary
0 siblings, 0 replies; 15+ messages in thread
From: hesham.almatary @ 2022-08-25 9:36 UTC (permalink / raw)
To: Rohit Mathew, devel@edk2.groups.io, Swatisri Kantamsetti
Cc: Sami Mujawar, Thomas Abraham, Thanu Rangarajan, nd
Hello Rohit,
On 8/24/2022 5:25 PM, Rohit Mathew wrote:
> Hi Hesham,
>
> The idea for keeping it as separate type was to abstract locator field to have a view similar to that of its definition in the spec (MPAM ACPI 1.0, section 2.2, table 7 - Resource node.)
>
> Something of the lines –
>
> typedef struct {
> UINT32 Identifier;
> UINT8 RisIndex;
> UINT16 Reserved1;
> UINT8 LocatorType;
> MPAM_LOCATOR Locator;
> UINT32 NumDependencies;
> } MPAM_MSC_RESOURCE;
>
> Locator field could very well be a struct of UINT64 and UINT32 with just descriptor 1 and descriptor 2 as the field names (Table 10, 2.3.2)– As you said, having hardcoded field names won’t work as the descriptors change with locator type.
>
>
> ///
>
> /// MPAM MSC Locator field
>
> ///
>
> typedef struct {
>
> UINT64 Descriptor1;
>
> UINT32 Descriptor2;
>
> } MPAM_LOCATOR;
>
> Let me know your thoughts on the approach.
Thanks for the explanation. It makes sense to me, I don't have a strong bias
towards not having it as a struct.
> Regards,
> Rohit
>
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of hesham.almatary via groups.io
> Sent: 24 August 2022 15:39
> To: Andrew Fish <afish@apple.com>; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
>
> Hello Rohit and Swatisri,
>
> On Tue, Aug 23, 2022 at 09:11 PM, Andrew Fish wrote:
> [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a separate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section 2.3.2 table 10 - locator descriptor)
> I'd just go for UINT64 locator1 and UINT32 locator2 and not necessarily a separate type. The interpretation of each depends on the the locator type (e.g., MPAM ACPI Tables 11-15).
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table
2022-08-16 20:18 [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Name
` (2 preceding siblings ...)
2022-08-23 17:52 ` Swatisri Kantamsetti
@ 2022-10-03 11:17 ` Sami Mujawar
3 siblings, 0 replies; 15+ messages in thread
From: Sami Mujawar @ 2022-10-03 11:17 UTC (permalink / raw)
To: Swatisri Kantamsetti, Name, devel, Alexei.Fedorov,
michael.d.kinney, gaoliming, zhiguang.liu
Cc: nd@arm.com, Rohit Mathew, Thanu.Rangarajan
[-- Attachment #1: Type: text/plain, Size: 6039 bytes --]
Hi Swatisri,
Apologies for the delay in feedback.
There is a new version of "ACPI for Memory System Resource Partitioning
and Monitoring 2.0, Platform Design Document" published on 2022/Sep/30
at https://developer.arm.com/documentation/den0065/latest.
The version 2.0 of ACPI MPAM table has a few errata fixes and some new
additions. Therefore, the version 1.0 of the ACPI MPAM table stands
deprecated and must not be used.
I would therefore suggest that this patch is updated to reflect ACPI
MPAM table version 2.0. I also have some feedback below that would still
apply.
Regards,
Sami Mujawar
On 16/08/2022 09:18 pm, Name wrote:
> From: Swatisri Kantamsetti<swatisrik@nvidia.com>
>
> Added MPAM table header, MSC and Resource Node
> info structures
>
> Signed-off-by: Swatisri Kantamsetti<swatisrik@nvidia.com>
> ---
> MdePkg/Include/IndustryStandard/Acpi64.h | 5 ++
> MdePkg/Include/IndustryStandard/Mpam.h | 69 ++++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
> create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
>
> diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
> index fe5ebfac2b..e54f631186 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi64.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi64.h
> @@ -2952,6 +2952,11 @@ typedef struct {
> ///
> #define EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('P', 'P', 'T', 'T')
>
> +///
> +/// "MPAM" Memory System Resource Partitioning And Monitoring Table
> +///
> +#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_STRUCTURE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M')
> +
> ///
> /// "PSDT" Persistent System Description Table
> ///
> diff --git a/MdePkg/Include/IndustryStandard/Mpam.h b/MdePkg/Include/IndustryStandard/Mpam.h
> new file mode 100644
> index 0000000000..e0f75f0114
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Mpam.h
> @@ -0,0 +1,69 @@
> +/** @file
> + ACPI Memory System Resource Partitioning And Monitoring (MPAM)
> + as specified in ARM spec DEN0065
> +
> + Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2022, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
[SAMI] Please add a '@par Specification Reference:' section as shown in
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5.3.7-include-files-shall-not-generate-code-or-define-data-variables.
e.g.
@par Specification Reference:
- [1] ACPI for Memory System Resource Partitioning and Monitoring 2.0,
Platform Design Document
(https://developer.arm.com/documentation/den0065/latest)
[/SAMI]
> +**/
> +
> +#ifndef _MPAM_H_
> +#define _MPAM_H_
[SAMI] I think the leading underscore should not be present, see
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/4_naming_conventions/43_identifiers#4.3.5.4-the-names-of-guard-macros-shall-end-with-an-underscore-character.
> +
> +#pragma pack(1)
> +
> +///
> +/// Memory System Resource Partitioning and Monitoring Table (MPAM)
> +///
> +typedef struct {
> + EFI_ACPI_DESCRIPTION_HEADER Header;
> + UINT32 NumNodes;
> + UINT32 NodeOffset;
> + UINT32 Reserved;
[SAMI] I do not see the NumNodes, NodeOffset and the Reserved filed in
the specification. Can you check, please?
> +} EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER;
[SAMI] I think the infix '_6_4_' is not going to work, as the ACPI MPAM
table specification is not going to be in sync with the ACPI
specification revision. So, this can be dropped.
Also, it may be better to add a @glossary section in the file header
describing the MPAM acronym and abbreviate the structure names to
EFI_ACPI_MPAM_TABLE_HEADER. The macro describing the table revision can
also follow a similar approach.
[/SAMI]
> +
> +///
> +/// MPAM Revision (as defined in ACPI 6.4 spec.)
[SAMI] I believe this should be (a defined by the 'ACPI for Memory
System Resource Partitioning and Monitoring, Platform Design Document').
Or simply refer it as '(as defined by [1])'.
> +///
> +#define EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_REVISION 0x01
[SAMI] Please move this definition before the definition of
EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORING_TABLE_HEADER.
The ACPI MPAM 1.0 table revision was 0. But for ACPI MPAM table 2.0 the
revision is 1, so there is no need to change.
> +
> +///
> +/// Memory System Controller Node Structure
> +///
> +
> +typedef struct {
> + UINT16 Length;
> + UINT16 Reserved;
[SAMI] ACPI MPAM 2.0 introduces splits the Reserved field to introduce
'Interface type'.
> + UINT32 Identifier;
> + UINT64 BaseAddress;
> + UINT32 MmioSize;
> + UINT32 OverflowInterrupt;
> + UINT32 OverflowInterruptFlags;
> + UINT32 Reserved1;
> + UINT32 OverflowInterruptAff;
> + UINT32 ErrorInterrupt;
> + UINT32 ErrorInterruptFlags;
> + UINT32 Reserved2;
> + UINT32 ErrorInterruptAff;
> + UINT32 MaxNRdyUsec;
> + UINT64 LinkedDeviceHwId;
> + UINT32 LinkedDeviceInstanceHwId;
> + UINT32 NumResourceNodes;
> +} EFI_ACPI_6_4_MPAM_MSC_NODE;
> +
> +///
> +/// Resource Node Structure
> +///
> +
> +typedef struct {
> + UINT32 Identifier;
> + UINT8 RisIndex;
> + UINT16 Reserved1;
> + UINT8 LocatorType;
> + UINT64 Locator;
[SAMI] Would it be possible to define a Locator structure with the first
field as the LocatorType followed by a union of Locators type specific
structures?
> + UINT32 NumFuncDep;
> +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE;
> +
[SAMI] Do you have plans to add definitions for the other structures
defined by the specifications, or the intention is to add them as required?
Also, if possible, it would be good to add an ACPI MPAM table parser to
Acpiview.
[/SAMI]
> +#pragma pack()
> +
> +#endif
[-- Attachment #2: Type: text/html, Size: 13740 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-10-03 11:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-16 20:18 [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Name
2022-08-16 20:18 ` [PATCH 2/2] Dynamic Tbl Mgr: MPAM: MPAM Generator and supporting files Name
2022-08-19 8:26 ` [edk2-devel] " Rohit Mathew
2022-08-24 10:51 ` Rohit Mathew
2022-08-23 17:51 ` Swatisri Kantamsetti
2022-08-19 8:26 ` [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Rohit Mathew
2022-08-23 20:10 ` Andrew Fish
2022-08-23 21:28 ` Rohit Mathew
2022-08-23 22:59 ` Andrew Fish
2022-08-24 10:42 ` Rohit Mathew
2022-08-24 14:39 ` hesham.almatary
2022-08-24 16:25 ` Rohit Mathew
2022-08-25 9:36 ` hesham.almatary
2022-08-23 17:52 ` Swatisri Kantamsetti
2022-10-03 11:17 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox