Thanks Sami, Michael, Liming for the review/ack. I'll post a V2 with the comments addressed. Let me also work on the Acpi parser for the same.
Regards, Rohit > -----Original Message----- > From: Sami Mujawar <sami.muja...@arm.com> > Sent: Wednesday, March 29, 2023 10:57 AM > 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>; Zhiguang Liu <zhiguang...@arm.com>; Thomas > Abraham <thomas.abra...@arm.com>; Swatisri Kantamsetti > <swatis...@nvidia.com>; nd <n...@arm.com> > Subject: Re: [edk2][PATCH V1 1/1] MdePkg/IndustryStandard: add definitions > for MPAM ACPI specification > > Hi Rohit, > > Thank you for this patch, and apologies for the delay in reply. > > I have a few minor suggestions marked inline as [SAMI]. > > Other than that, I would like to see a corresponding patch to add MPAM > support to Acpiview. > > With that addressed, > > Reviewed-by: Sami Mujawar <sami.muja...@arm.com> > > regards, > > Sami Mujawar > > On 25/01/2023 12:39 am, Rohit Mathew wrote: > > add definitions, macros and types for elements associated with MPAM > > ACPI 2.0 specification. > > > > Signed-off-by: Rohit Mathew <rohit.mat...@arm.com> > > --- > > MdePkg/Include/IndustryStandard/Acpi64.h | 7 +- > > MdePkg/Include/IndustryStandard/Mpam.h | 229 > ++++++++++++++++++++ > > 2 files changed, 235 insertions(+), 1 deletion(-) > > > > github link: > > https://github.com/rohit-arm/edk2/tree/mpam_acpi > > > > diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h > > b/MdePkg/Include/IndustryStandard/Acpi64.h > > index 575ca0430c..2be9525979 100644 > > --- a/MdePkg/Include/IndustryStandard/Acpi64.h > > +++ b/MdePkg/Include/IndustryStandard/Acpi64.h > > @@ -2,7 +2,7 @@ > > ACPI 6.4 definitions from the ACPI Specification Revision 6.4 Jan, 2021. > > > > 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 Limited. All rights reserved.<BR> > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > @@ -3157,6 +3157,11 @@ typedef struct { > > /// > > #define EFI_ACPI_6_4_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_SIG > > +NATURE 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..8e6e72ea28 > > --- /dev/null > > +++ b/MdePkg/Include/IndustryStandard/Mpam.h > > @@ -0,0 +1,229 @@ > > +/** @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_REV > > +ISION (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_PROCESSOR_AFFINITY (0x0) > > +#define EFI_ACPI_MPAM_PROCESSOR_CONTAINER_AFFINITY (0x1) > > + > > +/// > > +/// MPAM MSC affinity valid > > +/// > > +#define EFI_ACPI_MPAM_AFFINITY_NOT_VALID (0x0) > > +#define EFI_ACPI_MPAM_AFFINITY_VALID (0x1) > > + > > +/// > > +/// MPAM Interrupt flag - bit positions /// #define > > +EFI_ACPI_MPAM_INTERRUPT_MODE_POS (0x0) #define > > +EFI_ACPI_MPAM_INTERRUPT_TYPE_POS (0x1) > > +#define EFI_ACPI_MPAM_AFFINITY_TYPE_POS (0x3) > > +#define EFI_ACPI_MPAM_AFFINITY_VALID_POS (0x4) > > +#define EFI_ACPI_MPAM_RESERVED_POS (0x5) > [SAMI] I think it would be better to change the postfix _POS to _SHIFT and > corresponing definitions for masks > (i.e.EFI_ACPI_MPAM_INTERRUPT_MODE_MASK) be added. > > + > > +/// > > +/// 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 (0x0A) > > + > > +#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 HIDLinkedDevice; > [SAMI] I think the above field name should be HidLinkedDevice and similarly > the following field name should be InstanceIdLinkedDevice. > Otherwise I suspect the CI would complain. Can you check, please? > > + 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 NumDependencies; > > +} 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 (#102159): https://edk2.groups.io/g/devel/message/102159 Mute This Topic: https://groups.io/mt/96511685/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
