From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by mx.groups.io with SMTP id smtpd.web11.4685.1661295606100113193 for ; Tue, 23 Aug 2022 16:00:06 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=Iewwow64; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.42/8.16.0.42) with SMTP id 27NMwwqo035573; Tue, 23 Aug 2022 15:59:55 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=75CUtGj6mBs7aQGt3HvRc8UOhI0R5kcR3Jgqx2m9t3k=; b=Iewwow64YdEP6Pbar0ULRmZpm+RfPJ4/okDn7SFu8mH7QyJfZ2Ion9voWJLpZg0d/XzA I813OA4mGRX/nTZQxEa3LIvpX+5mGmE165nkIZ8U1mSpZzDY4e/kafKY5O+mW7jo7Gqu HRJuguZeeUR2L2R8lus4CX2PKdJKtGwzpi+LPrmlmmLxKeyEfpInm2wUFe1rm+nBwR0s ciIqeLJYaOek1KDY2bdqz1sGvFMSn9gymGCvfcHEmyGSRkRqdY6l2rQ4Qc1T9PvaaQbY ARRAqcC89JFy46ETdqwlWiBd3maAYabL5WG7RFjRRm7yc2A2i6eONGw0JG8+UjiHbVwP QA== Received: from rn-mailsvcp-mta-lapp03.rno.apple.com (rn-mailsvcp-mta-lapp03.rno.apple.com [10.225.203.151]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 3j2xkvw37a-6 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 23 Aug 2022 15:59:55 -0700 Received: from rn-mailsvcp-policy-lapp01.rno.apple.com (rn-mailsvcp-policy-lapp01.rno.apple.com [17.179.253.18]) by rn-mailsvcp-mta-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.19.20220711 64bit (built Jul 11 2022)) with ESMTPS id <0RH300JKMBVUD870@rn-mailsvcp-mta-lapp03.rno.apple.com>; Tue, 23 Aug 2022 15:59:54 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-policy-lapp01.rno.apple.com by rn-mailsvcp-policy-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.19.20220711 64bit (built Jul 11 2022)) id <0RH300T00BULF500@rn-mailsvcp-policy-lapp01.rno.apple.com>; Tue, 23 Aug 2022 15:59:54 -0700 (PDT) X-Va-A: X-Va-T-CD: cbd56c71540de3b82c6957096f91fbbb X-Va-E-CD: 900853de0c88d92a1a32a4de9c3fb6e3 X-Va-R-CD: 602562af33d08252d28a548f6df4870d X-Va-CD: 0 X-Va-ID: 8b83af38-d1dd-480c-b685-4ee91ce752c3 X-V-A: X-V-T-CD: cbd56c71540de3b82c6957096f91fbbb X-V-E-CD: 900853de0c88d92a1a32a4de9c3fb6e3 X-V-R-CD: 602562af33d08252d28a548f6df4870d X-V-CD: 0 X-V-ID: 6ef619d6-5892-4f9f-b26e-e57d0f83d144 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.895 definitions=2022-08-23_09:2022-08-22,2022-08-23 signatures=0 Received: from smtpclient.apple (unknown [17.235.47.162]) by rn-mailsvcp-policy-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.19.20220711 64bit (built Jul 11 2022)) with ESMTPSA id <0RH300TM1BVSQC00@rn-mailsvcp-policy-lapp01.rno.apple.com>; Tue, 23 Aug 2022 15:59:54 -0700 (PDT) From: "Andrew Fish" Message-id: MIME-version: 1.0 (Mac OS X Mail 16.0 \(3731.0.8.1.1\)) Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table Date: Tue, 23 Aug 2022 15:59:41 -0700 In-reply-to: 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 To: Rohit Mathew References: <7f8a5c9bbdf1a1f01c6fc822fa298067d280079a.1660667637.git.swatisrik@nvidia.com> X-Mailer: Apple Mail (2.3731.0.8.1.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.895 definitions=2022-08-23_09:2022-08-22,2022-08-23 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_07067E00-23AE-4631-9154-0096671C6892" --Apple-Mail=_07067E00-23AE-4631-9154-0096671C6892 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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 n= ot really want to encourage its use it in public interfaces.=20 Given there is lots of code 1st kind of things going on I=E2=80=99d figured= I=E2=80=99d mention this.=20 Thanks, Andrew Fish > On Aug 23, 2022, at 2:28 PM, Rohit Mathew wrote: >=20 > Hi Andrew, > =20 > From: Andrew Fish >=20 > Sent: 23 August 2022 21:11 > To: devel@edk2.groups.io ; Rohit Mathew > > Cc: username@nvidia.com ; Sami Mujawar >; Alexei Fedorov >; Mike Kinney >; gaoliming@byosoft.com.cn <= mailto:gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com ; Swatisri Kantamsetti >; Thomas Abraham >; Thanu Rangarajan >; nd > > Subject: Re: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Tabl= e > =20 > =20 >=20 >=20 > On Aug 19, 2022, at 1:26 AM, Rohit Mathew > wrote: > =20 > Hi Swatisri, >=20 > Thanks for the patch. Please find my comments inline marked [Rohit] - >=20 >=20 > -----Original Message----- > From: devel@edk2.groups.io > On Behalf Of Name > via groups.io > Sent: 16 August 2022 21:18 > To: devel@edk2.groups.io ; Sami Mujawar >; > Alexei Fedorov >; = michael.d.kinney@intel.com ; > gaoliming@byosoft.com.cn ; zhiguang.liu@= intel.com > Cc: Swatisri Kantamsetti > > Subject: [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table >=20 > From: Swatisri Kantamsetti > >=20 > Added MPAM table header, MSC and Resource Node info structures >=20 > Signed-off-by: Swatisri Kantamsetti > > --- > 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 >=20 > 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') >=20 > +/// > +/// "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; >=20 > [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, tab= le 4 - MSC Node body? >=20 >=20 > + > +/// > +/// 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; >=20 > [Rohit] Would it be better to have a type (possibly bitfield struct) inst= ead of a plain UINT32 for Flags? (MPAM ACPI 1.0, section 2.1.1, table 5 - I= nterrupt flags) >=20 > =20 > Probably better NOT to use bitfields in APIs that are produced and consum= ed by different worlds. While the the UEFI does speak to the bit order of o= r a bitfield the rules around packing of bitfields is compiler defined. > =20 > I just saw a bug last week with bitfield compatibility that was introduce= d by clang fixing its -mms-bitfields implementation. The GCC rules for pack= ing 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 a= nd it broke the code as the extra padding required by VC++ got added to the= bitfield.=20 > =20 > [Rohit] Thanks for bringing this point. I think, this type could be left = untouched in that case. > =20 > Thanks, > =20 > Andrew Fish >=20 >=20 > + UINT32 Reserved1; > + UINT32 OverflowInterruptAff; > + UINT32 ErrorInterrupt; > + UINT32 ErrorInterruptFlags; >=20 > [Rohit ] Same comment as before above. >=20 >=20 > + 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; >=20 > [Rohit ] Shouldn't " Locator " field be 12 bytes in size, possibly a sepa= rate type? (MPAM ACPI 1.0, section 2.2, table 7 - Resource node and section= 2.3.2 table 10 - locator descriptor) >=20 >=20 > + UINT32 NumFuncDep; > +} EFI_ACPI_6_4_MPAM_RESOURCE_NODE; >=20 > [Rohit] Since "NumFuncDep" field is part of EFI_ACPI_6_4_MPAM_RESOURCE_NO= DE type and this could be non-zero, should we also need the type for functi= onal dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, table 8 - Funct= ional dependency descriptor) >=20 > [Rohit] Also, could some of the commonly used macros be added to this hea= der, please? (location types, MPAM interrupt mode, interrupt types, affinit= y type, etc) >=20 >=20 > + > +#pragma pack() > + > +#endif > -- > 2.17.1 >=20 >=20 >=20 >=20 >=20 >=20 > Regards, > Rohit >=20 >=20 >=20 >=20 > =20 --Apple-Mail=_07067E00-23AE-4631-9154-0096671C6892 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Rohit,

FYI I s= eem 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 real= ly want to encourage its use it in public interfaces. 

<= /div>
Given there is lots of code 1st kind of things going on I=E2=80= =99d figured I=E2=80=99d mention this. 

Thank= s,

Andrew Fish

On Aug 23, 2022, at 2:28 PM, Rohit Mathew <rohit.mathew@arm.com&= gt; wrote:

Hi Andrew,
 
From: Andrew Fish <afish@apple.com> 
Sent: 23 August 2022 21:11
To: devel@edk2.g= roups.io; Rohit Mathew <Rohit.Mathew@arm.com>=
Cc: username@nvidia.com; Sami Mujawar <Sami.Mujawar= @arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.c= om>; Mike Kinney <michael.d.kinney@intel.co= m>; gaoliming@byosoft.com.cn;&n= bsp;zhiguang.liu@intel.com; Swatisri Kantamset= ti <swatisrik@nvidia.com>; Thomas Abraham <thomas.abraham@arm.com>; Thanu Rangarajan <Thanu.Rangarajan@arm.com>; nd <nd@arm.com&= gt;
Subject: Re= : [edk2-devel] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table<= /div>
 
 


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:<= span class=3D"apple-converted-space"> devel@edk= 2.groups.io <devel@edk2.groups.io> On Behalf Of Name
via groups.io
Sent: 16 Aug= ust 2022 21:18
To: devel@edk2.groups.io; Sami Mujawar <Sam= i.Mujawar@arm.com>;
Alexei Fedorov <<= span style=3D"font-size: 9pt; font-family: Helvetica, sans-serif;">Alexei.F= edorov@arm.com>; <= /span>michael.d.kinney@intel.com;
gaoliming@byosoft.com.cn; <= /span>zhiguang.liu@intel.com
Cc: Swatisri Kantamse= tti <swatisrik@nvidia.com>
Subject: [edk2-deve= l] [PATCH 1/2] Mde Pkg: Support for MPAM ACPI Table

From: Swatisri K= antamsetti <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/In= clude/IndustryStandard/Mpam.h   | 69
++++++++++++++++++++++++<= br>2 files changed, 74 insertions(+)
create mode 100644 MdePkg/Include/I= ndustryStandard/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 @@ typed= ef 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 Ta= ble
///
+#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 &n= bsp;/// 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 DEN= 0065
+
+  Copyright (c) 2022, NVIDIA CORPORATION. All rights res= erved.
+  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    H= eader;
+  UINT32         &n= bsp;            = ;   NumNodes;
+  UINT32     &nbs= p;            &= nbsp;      NodeOffset;
+  UINT32 &nbs= p;            &= nbsp;          Reserved;<= br>+}
+EFI_ACPI_6_4_MEMORY_SYSTEM_RESOURCE_PARTITIONING_MONITORI
NG_T= ABLE_HEADE
+R;


=
+  UINT32 &nb= sp;  Reserved1;
+  UINT32    OverflowInter= ruptAff;
+  UINT32    ErrorInterrupt;
+  UIN= T32    ErrorInterruptFlags;

[Rohit ] Same comment as before above.


+  UINT32   &nbs= p;Reserved2;
+  UINT32    ErrorInterruptAff;
+ &n= bsp;UINT32    MaxNRdyUsec;
+  UINT64   &nb= sp;LinkedDeviceHwId;
+  UINT32    LinkedDeviceInstan= ceHwId;
+  UINT32    NumResourceNodes;
+} EFI_ACP= I_6_4_MPAM_MSC_NODE;
+
+///
+/// Resource Node Structure
+///+
+typedef struct {
+  UINT32    Identifier;+  UINT8     RisIndex;
+  UINT16  &n= bsp; Reserved1;
+  UINT8     LocatorType;<= br>+  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 se= ction 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 typ= e for functional dependency descriptors? (MPAM ACPI 1.0, section 2.2.1, tab= le 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


 

--Apple-Mail=_07067E00-23AE-4631-9154-0096671C6892--