Hi Sami, Thank you for the review. Please find my comments inline.
Regards, Rohit From: Sami Mujawar <sami.muja...@arm.com> Sent: Monday, May 22, 2023 1:17 PM To: Rohit Mathew <rohit.mat...@arm.com>; devel@edk2.groups.io Cc: Michael D Kinney <michael.d.kin...@intel.com>; Liming Gao <gaolim...@byosoft.com.cn>; Thomas Abraham <thomas.abra...@arm.com>; James Morse <james.mo...@arm.com>; nd <n...@arm.com> Subject: Re: [PATCH V2 1/3] MdePkg/IndustryStandard: add definitions for MPAM ACPI specification Hi Rohit, Please find my response inline marked [SAMI]. Regards, Sami Mujawar On 18/05/2023 04:06 pm, Rohit Mathew wrote: From: Rohit Mathew <rohit.mat...@arm.com><mailto:rohit.mat...@arm.com> add definitions, macros and types for elements associated with MPAM ACPI 2.0 specification. Signed-off-by: Rohit Mathew <rohit.mat...@arm.com><mailto:rohit.mat...@arm.com> --- MdePkg/Include/IndustryStandard/Acpi65.h | 7 +- MdePkg/Include/IndustryStandard/Mpam.h | 258 ++++++++++++++++++++ 2 files changed, 264 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/IndustryStandard/Acpi65.h b/MdePkg/Include/IndustryStandard/Acpi65.h index 1e41ae9a27..8a1d3d125a 100644 --- a/MdePkg/Include/IndustryStandard/Acpi65.h +++ b/MdePkg/Include/IndustryStandard/Acpi65.h @@ -2,7 +2,7 @@ ACPI 6.5 definitions from the ACPI Specification Revision 6.5 Aug, 2022. Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR> - Copyright (c) 2019 - 2021, ARM Ltd. All rights reserved.<BR> + Copyright (c) 2019 - 2023, ARM Ltd. All rights reserved.<BR> Copyright (c) 2023, Loongson Technology Corporation Limited. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -3251,6 +3251,11 @@ typedef struct { /// #define EFI_ACPI_6_5_XEN_PROJECT_TABLE_SIGNATURE SIGNATURE_32('X', 'E', 'N', 'V') +/// +/// "MPAM" Memory System Resource Partitioning and Monitoring Table +/// +#define EFI_ACPI_MEMORY_SYSTEM_RESOURCE_PARTITIONING_AND_MONITORING_TABLE_SIGNATURE SIGNATURE_32('M', 'P', 'A', 'M') + #pragma pack() #endif diff --git a/MdePkg/Include/IndustryStandard/Mpam.h b/MdePkg/Include/IndustryStandard/Mpam.h new file mode 100644 index 0000000000..5d4c466abb --- /dev/null +++ b/MdePkg/Include/IndustryStandard/Mpam.h @@ -0,0 +1,258 @@ +/** @file + ACPI for Memory System Resource Partitioning and Monitoring 2.0 (MPAM) as + specified in ARM spec DEN0065 + + Copyright (c) 2023, Arm Limited. All rights reserved. + + SPDX-License-Identifier: BSD-2-Clause-Patent + + @par Specification Reference: + - [1] ACPI for Memory System Resource Partitioning and Monitoring 2.0 + (https://developer.arm.com/documentation/den0065/latest) + + @par Glossary: + - MPAM - Memory System Resource Partitioning And Monitoring + - MSC - Memory System Component + - PCC - Platform Communication Channel + - RIS - Resource Instance Selection + - SMMU - Arm System Memory Management Unit + **/ + +#ifndef MPAM_H_ +#define MPAM_H_ + +#include <IndustryStandard/Acpi.h> + +/// +/// MPAM Revision +/// +#define EFI_ACPI_MEMORY_SYSTEM_RESOURCE_PARTITIONING_AND_MONITORING_TABLE_REVISION (0x01) + +/// +/// MPAM Interrupt mode +/// +#define EFI_ACPI_MPAM_INTERRUPT_LEVEL_TRIGGERED (0x0) +#define EFI_ACPI_MPAM_INTERRUPT_EDGE_TRIGGERED (0x1) + +/// +/// MPAM Interrupt type +/// +#define EFI_ACPI_MPAM_INTERRUPT_WIRED (0x0) + +/// +/// MPAM Interrupt affinity type +/// +#define EFI_ACPI_MPAM_INTERRUPT_PROCESSOR_AFFINITY (0x0) +#define EFI_ACPI_MPAM_INTERRUPT_PROCESSOR_CONTAINER_AFFINITY (0x1) + +/// +/// MPAM MSC affinity valid +/// +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_NOT_VALID (0x0) +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID (0x1) + +/// +/// MPAM Interrupt flag - bit positions +/// +#define EFI_ACPI_MPAM_INTERRUPT_MODE_SHIFT (0) +#define EFI_ACPI_MPAM_INTERRUPT_TYPE_SHIFT (1) +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_TYPE_SHIFT (3) +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID_SHIFT (4) +#define EFI_ACPI_MPAM_INTERRUPT_RESERVED_SHIFT (5) + +/// +/// MPAM Interrupt flag - bit masks +/// +#define EFI_ACPI_MPAM_INTERRUPT_MODE_MASK (0x1) +#define EFI_ACPI_MPAM_INTERRUPT_TYPE_MASK (0x3) +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_TYPE_MASK (0x8) +#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID_MASK (0x10) +#define EFI_ACPI_MPAM_INTERRUPT_RESERVED_MASK (0xFFFFFFE0) + +/// +/// MPAM_MEMORY_LOCATOR_RESERVED_MASK should be used along with an 8 byte object +/// starting at the base of the locator field. +/// +#define EFI_ACPI_MPAM_MEM_LOCATOR_RESERVED_FIELD_MASK (0x00FFFFFFFFFFFFFFULL) +#define EFI_ACPI_MPAM_MEM_LOCATOR_RESERVED_FIELD_SHIFT (0) + +/// +/// MPAM_MEMORY_LOCATOR_LEVEL_MASK should be used along with an 8 byte object +/// starting at the base of the locator field. +/// +#define EFI_ACPI_MPAM_MEM_LOCATOR_LEVEL_FIELD_MASK (0xFF00000000000000ULL) +#define EFI_ACPI_MPAM_MEM_LOCATOR_LEVEL_FIELD_SHIFT (56) + [SAMI] Can you point me to the section in the spec where I can find this information, please? Are these definitions for Memory-side cache locator descriptor by any chance? [Rohit] These are from mem cache locator field, like you pointed (Section 2.3.3.4 – table 16). I’ll rename these to EFI_ACPI_MPAM_MEM_CACHE_* if the masks and shifts look fine. +/// +/// MPAM Location types +/// as described in document [1], table 11 +/// +#define EFI_ACPI_MPAM_LOCATION_PROCESSOR_CACHE (0x0) +#define EFI_ACPI_MPAM_LOCATION_MEMORY (0x1) +#define EFI_ACPI_MPAM_LOCATION_SMMU (0x2) +#define EFI_ACPI_MPAM_LOCATION_MEMORY_CACHE (0x3) +#define EFI_ACPI_MPAM_LOCATION_ACPI_DEVICE (0x4) +#define EFI_ACPI_MPAM_LOCATION_INTERCONNECT (0x5) +#define EFI_ACPI_MPAM_LOCATION_UNKNOWN (0xFF) + +/// +/// MPAM Interface types +/// +#define EFI_ACPI_MPAM_INTERFACE_MMIO (0x0) +#define EFI_ACPI_MPAM_INTERFACE_PCC (0x1) + +/// +/// MPAM Link types +/// +#define EFI_ACPI_MPAM_LINK_TYPE_NUMA (0x0) +#define EFI_ACPI_MPAM_LINK_TYPE_PROC (0x0A) [SAMI] Is the EFI_ACPI_MPAM_LINK_TYPE_PROC values and EFI_ACPI_MPAM_INTERFACE_PCC value swapped by any chance? Or I am looking at a completely wrong spec. Can you point me to the correct spec, please? [/SAMI] [Rohit] Ah! That indeed got swapped around. Let me fix this in the next revision. Do you want me to send out a new revision for this patch alone (or I can hold until the series is reviewed). PS: We are both looking at the spec spec revision. [/Rohit] + +#pragma pack(1) + +/// +/// MPAM MSC generic locator descriptor +/// as described in document [1], table 12 +/// +typedef struct { + UINT64 Descriptor1; + UINT32 Descriptor2; +} EFI_ACPI_MPAM_GENERIC_LOCATOR; + +/// +/// MPAM processor cache locator descriptor +/// as described in document [1], table 13 +/// +typedef struct { + UINT64 CacheReference; + UINT32 Reserved; +} EFI_ACPI_MPAM_CACHE_LOCATOR; + +/// +/// MPAM memory locator descriptor +/// as described in document [1], table 14 +/// +typedef struct { + UINT64 ProximityDomain; + UINT32 Reserved; +} EFI_ACPI_MPAM_MEMORY_LOCATOR; + +/// +/// MPAM SMMU locator descriptor +/// as described in document [1], table 15 +/// +typedef struct { + UINT64 SmmuInterface; + UINT32 Reserved; +} EFI_ACPI_MPAM_SMMU_LOCATOR; + +/// +/// MPAM memory-side cache locator descriptor +/// as described in Document [1], table 16 +/// +typedef struct { + UINT8 Reserved[7]; + UINT8 Level; + UINT32 Reference; +} EFI_ACPI_MPAM_MEMORY_CACHE_LOCATOR; + +/// +/// MPAM ACPI device locator descriptor +/// as described in document [1], table 17 +/// +typedef struct { + UINT64 AcpiHardwareId; + UINT32 AcpiUniqueId; +} EFI_ACPI_MPAM_ACPI_LOCATOR; + +/// +/// MPAM interconnect locator descriptor +/// as described in document [1], table 18 +/// +typedef struct { + UINT64 InterconnectDescTblOff; + UINT32 Reserved; +} EFI_ACPI_MPAM_INTERCONNECT_LOCATOR; + +/// +/// MPAM interconnect descriptor +/// as described in document [1], table 19 +/// +typedef struct { + UINT32 SourceId; + UINT32 DestinationId; + UINT8 LinkType; + UINT8 Reserved[3]; +} EFI_ACPI_MPAM_INTERCONNECT_DESCRIPTOR; + +/// +/// MPAM interconnect descriptor table +/// as described in document [1], table 20 +/// +typedef struct { + UINT8 Signature[16]; + UINT32 NumDescriptors; +} EFI_ACPI_MPAM_INTERCONNECT_DESCRIPTOR_TABLE; + +/// +/// MPAM resource locator +/// +typedef union { + EFI_ACPI_MPAM_CACHE_LOCATOR CacheLocator; + EFI_ACPI_MPAM_MEMORY_LOCATOR MemoryLocator; + EFI_ACPI_MPAM_SMMU_LOCATOR SmmuLocator; + EFI_ACPI_MPAM_MEMORY_CACHE_LOCATOR MemCacheLocator; + EFI_ACPI_MPAM_ACPI_LOCATOR AcpiLocator; + EFI_ACPI_MPAM_INTERCONNECT_LOCATOR InterconnectIfcLocator; + EFI_ACPI_MPAM_GENERIC_LOCATOR GenericLocator; +} EFI_ACPI_MPAM_LOCATOR; + +/// +/// MPAM MSC node body +/// as described document [1], table 4 +/// +typedef struct { + UINT16 Length; + UINT8 InterfaceType; + UINT8 Reserved; + UINT32 Identifier; + UINT64 BaseAddress; + UINT32 MmioSize; + UINT32 OverflowInterrupt; + UINT32 OverflowInterruptFlags; + UINT32 Reserved1; + UINT32 OverflowInterruptAffinity; + UINT32 ErrorInterrupt; + UINT32 ErrorInterruptFlags; + UINT32 Reserved2; + UINT32 ErrorInterruptAffinity; + UINT32 MaxNrdyUsec; + UINT64 HardwareIdLinkedDevice; + UINT32 InstanceIdLinkedDevice; + UINT32 NumResources; +} EFI_ACPI_MPAM_MSC_NODE; + +/// +/// MPAM MSC resource +/// as described in document [1], table 9 +/// +typedef struct { + UINT32 Identifier; + UINT8 RisIndex; + UINT16 Reserved1; + UINT8 LocatorType; + EFI_ACPI_MPAM_LOCATOR Locator; + UINT32 NumFunctionalDependencies; +} EFI_ACPI_MPAM_MSC_RESOURCE; + +/// +/// MPAM Function dependency descriptor +/// as described in document [1], table 10 +/// +typedef struct { + UINT32 Producer; + UINT32 Reserved; +} EFI_ACPI_MPAM_FUNCTIONAL_DEPENDENCY_DESCRIPTOR; + +#pragma pack() + +#endif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105123): https://edk2.groups.io/g/devel/message/105123 Mute This Topic: https://groups.io/mt/98993032/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-