public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4 0/8] IORT Rev E.d specification updates
@ 2022-07-06  9:56 Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, ardb+tianocore, quic_llindhol,
	pierre.gondois, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	steven.price, Lorenzo.Pieralisi, michael.d.kinney, gaoliming,
	zhiguang.liu, ray.ni, zhichao.gao, nd

Bugzilla: 3458 - Add support IORT Rev E.d specification updates
          (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

The IO Remapping Table, Platform Design Document, Revision E.d,
Feb 2022 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
  - increments the IORT table revision to 5.
  - updates the node definition to add an 'Identifier' field.
  - adds definition of node type 6 - Reserved Memory Range node.
  - adds definition for Memory Range Descriptors.
  - adds flag to indicate PRI support for root complexes.
  - adds flag to indicate if the root complex supports forwarding
    of PASID information on translated transactions to the SMMU.
  - adds flag to indicate if the root complex supports PASID.
  - adds flags to define access privilege and attributes for the
    memory ranges.

This v4 series:
 * Updates the IORT header to reflect Rev E.d:
   - Add flag to indicate if the root complex supports PASID.
   - Add flags to define access privilege and attributes for the
     memory ranges.
 * Adds validations to Acpiview to:
   - Check that the IORT Table Revision is not 4 as IORT
     Rev E.c is deprecated.
   - Check that the IORT RMR node revision is not 2 as it
     breaks backward compatibility and was deprecated as
     part of IORT Rev E.c.
 * Updates Dynamic Tables Framework to support generation of
     IORT revision E.d.

The v3 series:
 - Dropped [PATCH v2 1/8] MdePkg: Fix IORT header file include
   guard as suggested at
   https://edk2.groups.io/g/devel/message/76656
 - Removed definition of EFI_ACPI_IO_REMAPPING_TABLE_REVISION as
   EFI_ACPI_IO_REMAPPING_TABLE_REV0 has been provided for
   representing Rev 0.
 - Moved error handling code for IdMappingToken from patch v2 6/8
   and v2 8/8 into a separate patch.
 - Moved Identifier field before Flags field in CM_ARM_RMR_NODE.
 - Added description for CM_ARM_MEMORY_RANGE_DESCRIPTOR field.

The v2 patch series includes all changes from v1 patch series
except the following 2 patches have been modified to set the
EFI_ACPI_IO_REMAPPING_TABLE_REVISION macro to Rev 0 as setting
to Rev 3 will break existing platforms, the problem being that
the Identifier field in the IORT nodes would not be unique.
  - MdePkg: IORT header update for IORT Rev E.b spec
  - DynamicTablesPkg: IORT generator updates for Rev E.b spec

The v1 patch series:
  - Updates the IORT header file to match the Rev E.b specification.
  - Add support to parse IORT Rev E.b tables
  - Add support to generate IORT Rev E.b compliant ACPI tables
    using Dynamic Tables Framework.

The changes for the v4 series can be seen at:
https://github.com/samimujawar/edk2/tree/1527_iort_rev_ed_v4
Sami Mujawar (8):
  MdePkg: IORT header update for IORT Rev E.d spec
  ShellPkg: Acpiview: Abbreviate field names to preserve alignment
  ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec
  DynamicTablesPkg: Handle error when IdMappingToken is NULL
  DynamicTablesPkg: IORT set reference to Id array only if present
  DynamicTablesPkg: IORT set reference to interrupt array if present
  DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.d
  DynamicTablesPkg: IORT generator updates for Rev E.d spec

 DynamicTablesPkg/DynamicTablesPkg.ci.yaml                              |   1 +
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h                         |  66 +-
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c       | 826 +++++++++++++++++---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h       |   5 +-
 MdePkg/Include/IndustryStandard/IoRemappingTable.h                     |  83 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 230 +++++-
 6 files changed, 1071 insertions(+), 140 deletions(-)

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06 15:44   ` [edk2-devel] " Sami Mujawar
  2022-07-07  6:05   ` Jon Nettleton
  2022-07-06  9:56 ` [PATCH v4 2/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment Sami Mujawar
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, ardb+tianocore, quic_llindhol,
	pierre.gondois, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	steven.price, Lorenzo.Pieralisi, michael.d.kinney, gaoliming,
	zhiguang.liu, ray.ni, zhichao.gao, nd

Bugzilla: 3458 - Add support IORT Rev E.d specification updates
          (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

The IO Remapping Table, Platform Design Document, Revision E.d,
Feb 2022 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
  - increments the IORT table revision to 5.
  - updates the node definition to add an 'Identifier' field.
  - adds definition of node type 6 - Reserved Memory Range node.
  - adds definition for Memory Range Descriptors.
  - adds flag to indicate PRI support for root complexes.
  - adds flag to indicate if the root complex supports forwarding
    of PASID information on translated transactions to the SMMU.
  - adds flag to indicate if the root complex supports PASID.
  - adds flags to define access privilege and attributes for the
    memory ranges.

Therefore, update the IORT header file to reflect these changes,
and also rename the EFI_ACPI_IO_REMAPPING_TABLE_REVISION macro to
EFI_ACPI_IO_REMAPPING_TABLE_REV0.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
      - Updated patch series to IORT specification revision E.d.     [SAMI]
      - Add flag to indicate if the root complex supports PASID.     [SAMI]
      - Add flags to define access privilege and attributes for      [SAMI]
        the memory ranges.
    v3:
      - Submit patch series to update platform code to use the       [LIMING]
        EFI_ACPI_IO_REMAPPING_TABLE_REV0 macro.
        Ref: https://edk2.groups.io/g/devel/topic/83618423#76799
      - Removed definition of EFI_ACPI_IO_REMAPPING_TABLE_REVISION   [SAMI]
        as EFI_ACPI_IO_REMAPPING_TABLE_REV0 has been provided for
        representing Rev 0. Also, a corresponding patch series
        for updating the platforms in edk2-platforms repository
        shall be submitted to the edk2 mailing list.
      - Include r-b received from v2 series.                         [SAMI]
        Ref: https://edk2.groups.io/g/devel/topic/83600724#76660
    
    v2:
      - Set EFI_ACPI_IO_REMAPPING_TABLE_REVISION to Rev 0 as     [SAMI]
        setting to Rev 3 will break existing platforms. The
        problem is that existing code would not be populating
        the Identifier field in the nodes. This can lead to
        non-unique values in the Identifier field.

 MdePkg/Include/IndustryStandard/IoRemappingTable.h | 83 ++++++++++++++++++--
 1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
index 79a34678681d45b2982dc8573db6bd447f42e429..07cb7f49dc936fb00cc549113f1e62f988535e5d 100644
--- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
+++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
@@ -1,12 +1,19 @@
 /** @file
-  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049D
-
-  http://infocenter.arm.com/help/topic/com.arm.doc.den0049d/DEN0049D_IO_Remapping_Table.pdf
+  ACPI IO Remapping Table (IORT) definitions.
 
   Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
-  Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
+    (https://developer.arm.com/documentation/den0049/)
+
+  @par Glossary:
+  - Ref  : Reference
+  - Mem  : Memory
+  - Desc : Descriptor
 **/
 
 #ifndef __IO_REMAPPING_TABLE_H__
@@ -14,7 +21,8 @@
 
 #include <IndustryStandard/Acpi.h>
 
-#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  0x0
+#define EFI_ACPI_IO_REMAPPING_TABLE_REV0  0x0
+#define EFI_ACPI_IO_REMAPPING_TABLE_REV5  0x5
 
 #define EFI_ACPI_IORT_TYPE_ITS_GROUP     0x0
 #define EFI_ACPI_IORT_TYPE_NAMED_COMP    0x1
@@ -22,6 +30,7 @@
 #define EFI_ACPI_IORT_TYPE_SMMUv1v2      0x3
 #define EFI_ACPI_IORT_TYPE_SMMUv3        0x4
 #define EFI_ACPI_IORT_TYPE_PMCG          0x5
+#define EFI_ACPI_IORT_TYPE_RMR           0x6
 
 #define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA  BIT0
 
@@ -55,7 +64,29 @@
 #define EFI_ACPI_IORT_SMMUv3_MODEL_CAVIUM_CN99XX     0x2
 
 #define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
-#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
+#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    BIT0
+
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_UNSUPPORTED  0x0
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_SUPPORTED    BIT1
+
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_UNSUPPORTED  0x0
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_SUPPORTED    BIT2
+
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_UNSUPPORTED  0x0
+#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_SUPPORTED    BIT1
+
+#define EFI_ACPI_IORT_RMR_REMAP_NOT_PERMITTED  0x0
+#define EFI_ACPI_IORT_RMR_REMAP_PERMITTED      BIT0
+
+#define EFI_ACPI_IORT_RMR_ACCESS_REQ_NOT_PRIVILEGED  0x0
+#define EFI_ACPI_IORT_RMR_ACCESS_REQ_PRIVILEGED      BIT1
+
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRNE             0x0
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRE              0x1
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGRE               0x2
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_GRE                0x3
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_NC_OUT_NC      0x4
+#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_WB_OUT_WB_ISH  0x5
 
 #define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE  BIT0
 
@@ -89,7 +120,7 @@ typedef struct {
   UINT8     Type;
   UINT16    Length;
   UINT8     Revision;
-  UINT32    Reserved;
+  UINT32    Identifier;
   UINT32    NumIdMappings;
   UINT32    IdReference;
 } EFI_ACPI_6_0_IO_REMAPPING_NODE;
@@ -118,7 +149,9 @@ typedef struct {
   UINT32                            AtsAttribute;
   UINT32                            PciSegmentNumber;
   UINT8                             MemoryAddressSize;
-  UINT8                             Reserved1[3];
+  UINT16                            PasidCapabilities;
+  UINT8                             Reserved1[1];
+  UINT32                            Flags;
 } EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
 
 ///
@@ -198,6 +231,40 @@ typedef struct {
   // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE      OverflowInterruptMsiMapping[1];
 } EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE;
 
+///
+/// Memory Range Descriptor.
+///
+typedef struct {
+  /// Base address of Reserved Memory Range,
+  /// aligned to a page size of 64K.
+  UINT64    Base;
+
+  /// Length of the Reserved Memory range.
+  /// Must be a multiple of the page size of 64K.
+  UINT64    Length;
+
+  /// Reserved, must be zero.
+  UINT32    Reserved;
+} EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC;
+
+///
+/// Node type 6: Reserved Memory Range (RMR) node
+///
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE    Node;
+
+  /// RMR flags
+  UINT32                            Flags;
+
+  /// Memory range descriptor count.
+  UINT32                            NumMemRangeDesc;
+
+  /// Offset of the memory range descriptor array.
+  UINT32                            MemRangeDescRef;
+  // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE         IdMapping[1];
+  // EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC   MemRangeDesc[1];
+} EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE;
+
 #pragma pack()
 
 #endif
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH 3/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec Sami Mujawar
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, ray.ni, zhichao.gao, nd

Some field names in the IORT table parser were longer than the
OUTPUT_FIELD_COLUMN_WIDTH plus indentation, resulting in loss of
the output print alignment. Therefore, abbreviate the field names.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---

Notes:
    v4:
      - No code change since v1. Including r-b and resending   [SAMI]
        with v4 series.
    
    v3:
      - No code change since v1. Include r-b received          [SAMI]
        from v2 series.
        Ref: https://edk2.groups.io/g/devel/topic/83600717#7665
    v2:
      - No code change since v1. Re-sending with v2 series.    [SAMI]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 81bfacd83added87a867cf365a56d4b7a1410ef2..44d633c5282463078a4cc990bb24ca1992f95634 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -1,11 +1,14 @@
 /** @file
   IORT table parser
 
-  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-    - IO Remapping Table, Platform Design Document, Revision D, March 2018
+  - IO Remapping Table, Platform Design Document, Revision D, March 2018
+
+  @par Glossary:
+    - Ref  - Reference
 **/
 
 #include <IndustryStandard/IoRemappingTable.h>
@@ -144,15 +147,15 @@ STATIC CONST ACPI_PARSER  IortNodeSmmuV1V2Parser[] = {
   { L"Span",                          8,    24,  L"0x%lx", NULL, NULL, NULL, NULL },
   { L"Model",                         4,    32,  L"%d",    NULL, NULL, NULL, NULL },
   { L"Flags",                         4,    36,  L"0x%x",  NULL, NULL, NULL, NULL },
-  { L"Reference to Global Interrupt Array",4,    40,  L"0x%x",  NULL, NULL, NULL,
+  { L"Global Interrupt Array Ref",    4,    40,  L"0x%x",  NULL, NULL, NULL,
     NULL },
   { L"Number of context interrupts",  4,    44,  L"%d",    NULL,
     (VOID **)&InterruptContextCount,  NULL, NULL },
-  { L"Reference to Context Interrupt Array",4,    48,  L"0x%x",  NULL,
+  { L"Context Interrupt Array Ref",   4,    48,  L"0x%x",  NULL,
     (VOID **)&InterruptContextOffset, NULL, NULL },
   { L"Number of PMU Interrupts",      4,    52,  L"%d",    NULL,
     (VOID **)&PmuInterruptCount,      NULL, NULL },
-  { L"Reference to PMU Interrupt Array",4,    56,  L"0x%x",  NULL,
+  { L"PMU Interrupt Array Ref",       4,    56,  L"0x%x",  NULL,
     (VOID **)&PmuInterruptOffset,     NULL, NULL },
 
   // Interrupt Array
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 2/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 4/8] DynamicTablesPkg: Handle error when IdMappingToken is NULL Sami Mujawar
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, steven.price,
	Lorenzo.Pieralisi, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	ray.ni, zhichao.gao, nd

Bugzilla: 3458 - Add support IORT Rev E.d specification updates
          (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

The IO Remapping Table, Platform Design Document, Revision E.d,
Feb 2022 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
 - increments the IORT table revision to 5.
 - updates the node definition to add an 'Identifier' field.
 - adds definition of node type 6 - Reserved Memory Range node.
 - adds definition for Memory Range Descriptors.
 - adds flag to indicate PRI support for root complexes.
 - adds flag to indicate if the root complex supports forwarding
   of PASID information on translated transactions to the SMMU.
 - adds flag to indicate if the root complex supports PASID.
 - adds flags to define access privilege and attributes for the
   memory ranges.

Therefore, update the IORT parser to:
  - parse the Identifier field.
  - parse Reserved Memory Range node.
  - parse Memory Range Descriptors.
  - add validations to check that the physical range base
    and size of the Memory Range Descriptor is 64KB aligned.
  - add validation to check that the IORT Table Revision is
    not 4 as IORT Rev E.c is deprecated.
  - add validation to check that the IORT RMR node revision
    is not 2 as it breaks backward compatibility and was
    deprecated as part of IORT Rev E.c.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
     - Add validation to check that the IORT Table Revision is  [SAMI]
       not 4 as IORT Rev E.c is deprecated.
     - Add validation to check that the IORT RMR node revision  [SAMI]
       is not 2 as it breaks backward compatibility and was
       deprecated as part of IORT Rev E.c.
    
    v3:
     - No code change since v1. Included r-b received for       [SAMI]
       v2 series.
       Ref: https://edk2.groups.io/g/devel/topic/83600720#7665
    
    v2:
      - No code change since v1. Re-sending with v2 series.     [SAMI]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 221 ++++++++++++++++++--
 1 file changed, 203 insertions(+), 18 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 44d633c5282463078a4cc990bb24ca1992f95634..0b42ff3b354a7cf54cdc2e6e15d0617fadda17f2 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -1,14 +1,16 @@
 /** @file
   IORT table parser
 
-  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2016 - 2022, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-  - IO Remapping Table, Platform Design Document, Revision D, March 2018
+    - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
+      (https://developer.arm.com/documentation/den0049/)
 
   @par Glossary:
     - Ref  - Reference
+    - Desc - Descriptor
 **/
 
 #include <IndustryStandard/IoRemappingTable.h>
@@ -26,6 +28,7 @@ STATIC CONST UINT32  *IortNodeOffset;
 
 STATIC CONST UINT8   *IortNodeType;
 STATIC CONST UINT16  *IortNodeLength;
+STATIC CONST UINT8   *IortNodeRevision;
 STATIC CONST UINT32  *IortIdMappingCount;
 STATIC CONST UINT32  *IortIdMappingOffset;
 
@@ -36,6 +39,9 @@ STATIC CONST UINT32  *PmuInterruptOffset;
 
 STATIC CONST UINT32  *ItsCount;
 
+STATIC CONST UINT32  *RmrMemDescCount;
+STATIC CONST UINT32  *RmrMemDescOffset;
+
 /**
   This function validates the ID Mapping array count for the ITS node.
 
@@ -100,6 +106,52 @@ ValidateItsIdArrayReference (
   }
 }
 
+/**
+  This function validates that the Physical Range address or length is not zero
+  and is 64K aligned.
+
+  @param [in] Ptr     Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+                      could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidatePhysicalRange (
+  IN UINT8  *Ptr,
+  IN VOID   *Context
+  )
+{
+  UINT64  Value;
+
+  Value = *(UINT64 *)Ptr;
+  if ((Value == 0) || ((Value & (SIZE_64KB - 1)) != 0)) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: Physical Range must be 64K aligned and cannot be zero.");
+  }
+}
+
+/**
+  This function validates that the RMR memory range descriptor count.
+
+  @param [in] Ptr     Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+                      could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateRmrMemDescCount (
+  IN UINT8  *Ptr,
+  IN VOID   *Context
+  )
+{
+  if (*(UINT32 *)Ptr == 0) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: Memory Range Descriptor count must be >=1.");
+  }
+}
+
 /**
   Helper Macro for populating the IORT Node header in the ACPI_PARSER array.
 
@@ -108,15 +160,15 @@ ValidateItsIdArrayReference (
   @param [out] ValidateIdArrayReference  Optional pointer to a function for
                                          validating the ID Array reference.
 **/
-#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount,                   \
-                               ValidateIdArrayReference)                 \
-  { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL },     \
-  { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \
-  { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL },                  \
-  { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL },                \
-  { L"Number of ID mappings", 4, 8, L"%d", NULL,                         \
-    (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL },         \
-  { L"Reference to ID Array", 4, 12, L"0x%x", NULL,                      \
+#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount,                        \
+                               ValidateIdArrayReference)                      \
+  { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL },          \
+  { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL },      \
+  { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL, NULL },  \
+  { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL },                   \
+  { L"Number of ID mappings", 4, 8, L"%d", NULL,                              \
+    (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL },              \
+  { L"Reference to ID Array", 4, 12, L"0x%x", NULL,                           \
     (VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL }
 
 /**
@@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER  IortNodeNamedComponentParser[] = {
 **/
 STATIC CONST ACPI_PARSER  IortNodeRootComplexParser[] = {
   PARSE_IORT_NODE_HEADER (NULL, NULL),
-  { L"Memory access properties",8,    16,  L"0x%lx",    NULL,       NULL, NULL, NULL },
-  { L"ATS Attribute",           4,    24,  L"0x%x",     NULL,       NULL, NULL, NULL },
-  { L"PCI Segment number",      4,    28,  L"0x%x",     NULL,       NULL, NULL, NULL },
-  { L"Memory access size limit",1,    32,  L"0x%x",     NULL,       NULL, NULL, NULL },
-  { L"Reserved",                3,    33,  L"%x %x %x", Dump3Chars, NULL, NULL, NULL }
+  { L"Memory access properties",8,    16,  L"0x%lx", NULL, NULL, NULL, NULL },
+  { L"ATS Attribute",           4,    24,  L"0x%x",  NULL, NULL, NULL, NULL },
+  { L"PCI Segment number",      4,    28,  L"0x%x",  NULL, NULL, NULL, NULL },
+  { L"Memory access size limit",1,    32,  L"0x%x",  NULL, NULL, NULL, NULL },
+  { L"PASID capabilities",      2,    33,  L"0x%x",  NULL, NULL, NULL, NULL },
+  { L"Reserved",                1,    35,  L"%x",    NULL, NULL, NULL, NULL },
+  { L"Flags",                   4,    36,  L"%x",    NULL, NULL, NULL, NULL },
 };
 
 /**
@@ -253,6 +307,29 @@ STATIC CONST ACPI_PARSER  IortNodePmcgParser[] = {
   { L"Page 1 Base Address",                           8,    32,  L"0x%lx", NULL, NULL, NULL, NULL }
 };
 
+/**
+  An ACPI_PARSER array describing the IORT RMR node.
+**/
+STATIC CONST ACPI_PARSER  IortNodeRmrParser[] = {
+  PARSE_IORT_NODE_HEADER (NULL, NULL),
+  { L"Flags",                   4,                      16,  L"0x%x", NULL, NULL, NULL, NULL },
+  { L"Memory Range Desc count", 4,                      20,  L"%d",   NULL,
+    (VOID **)&RmrMemDescCount,  ValidateRmrMemDescCount,NULL },
+  { L"Memory Range Desc Ref",   4,                      24,  L"0x%x", NULL,
+    (VOID **)&RmrMemDescOffset, NULL,                   NULL }
+};
+
+/**
+  An ACPI_PARSER array describing the IORT RMR Memory Range Descriptor.
+**/
+STATIC CONST ACPI_PARSER  IortNodeRmrMemRangeDescParser[] = {
+  { L"Physical Range offset", 8, 0,  L"0x%lx", NULL, NULL, ValidatePhysicalRange,
+    NULL },
+  { L"Physical Range length", 8, 8,  L"0x%lx", NULL, NULL, ValidatePhysicalRange,
+    NULL },
+  { L"Reserved",              4, 16, L"0x%x",  NULL, NULL, NULL,                 NULL}
+};
+
 /**
   This function parses the IORT Node Id Mapping array.
 
@@ -607,9 +684,102 @@ DumpIortNodePmcg (
     );
 }
 
+/**
+  This function parses the IORT RMR Node Memory Range Descriptor array.
+
+  @param [in] Ptr         Pointer to the start of the Memory Range Descriptor
+                          array.
+  @param [in] Length      Length of the buffer.
+  @param [in] DescCount   Memory Range Descriptor count.
+**/
+STATIC
+VOID
+DumpIortNodeRmrMemRangeDesc (
+  IN UINT8   *Ptr,
+  IN UINT32  Length,
+  IN UINT32  DescCount
+  )
+{
+  UINT32  Index;
+  UINT32  Offset;
+  CHAR8   Buffer[40]; // Used for AsciiName param of ParseAcpi
+
+  Index  = 0;
+  Offset = 0;
+
+  while ((Index < DescCount) &&
+         (Offset < Length))
+  {
+    AsciiSPrint (
+      Buffer,
+      sizeof (Buffer),
+      "Mem range Descriptor [%d]",
+      Index
+      );
+    Offset += ParseAcpi (
+                TRUE,
+                4,
+                Buffer,
+                Ptr + Offset,
+                Length - Offset,
+                PARSER_PARAMS (IortNodeRmrMemRangeDescParser)
+                );
+    Index++;
+  }
+}
+
+/**
+  This function parses the IORT RMR node.
+
+  @param [in] Ptr            Pointer to the start of the buffer.
+  @param [in] Length         Length of the buffer.
+  @param [in] MappingCount   The ID Mapping count.
+  @param [in] MappingOffset  The offset of the ID Mapping array
+                             from the start of the IORT table.
+**/
+STATIC
+VOID
+DumpIortNodeRmr (
+  IN UINT8   *Ptr,
+  IN UINT16  Length,
+  IN UINT32  MappingCount,
+  IN UINT32  MappingOffset
+  )
+{
+  ParseAcpi (
+    TRUE,
+    2,
+    "RMR Node",
+    Ptr,
+    Length,
+    PARSER_PARAMS (IortNodeRmrParser)
+    );
+
+  if (*IortNodeRevision == 2) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: RMR node Rev 2 (defined in IORT Rev E.c) must not be used."
+      L" IORT tabe Revision E.c is deprecated and must not be used.\n"
+      );
+  }
+
+  DumpIortNodeIdMappings (
+    Ptr + MappingOffset,
+    Length - MappingOffset,
+    MappingCount
+    );
+
+  DumpIortNodeRmrMemRangeDesc (
+    Ptr + (*RmrMemDescOffset),
+    Length - (*RmrMemDescOffset),
+    *RmrMemDescCount
+    );
+}
+
 /**
   This function parses the ACPI IORT table.
-  When trace is enabled this function parses the IORT table and traces the ACPI fields.
+  When trace is enabled this function parses the IORT table and traces the ACPI
+  fields.
 
   This function also parses the following nodes:
     - ITS Group
@@ -618,6 +788,7 @@ DumpIortNodePmcg (
     - SMMUv1/2
     - SMMUv3
     - PMCG
+    - RMR
 
   This function also performs validation of the ACPI table fields.
 
@@ -643,6 +814,13 @@ ParseAcpiIort (
     return;
   }
 
+  if (AcpiTableRevision == 4) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: IORT tabe Revision E.c is deprecated and must not be used.\n"
+      );
+  }
+
   ParseAcpi (
     TRUE,
     0,
@@ -765,7 +943,14 @@ ParseAcpiIort (
           *IortIdMappingOffset
           );
         break;
-
+      case EFI_ACPI_IORT_TYPE_RMR:
+        DumpIortNodeRmr (
+          NodePtr,
+          *IortNodeLength,
+          *IortIdMappingCount,
+          *IortIdMappingOffset
+          );
+        break;
       default:
         IncrementErrorCount ();
         Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/8] DynamicTablesPkg: Handle error when IdMappingToken is NULL
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
                   ` (2 preceding siblings ...)
  2022-07-06  9:56 ` [PATCH 3/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 5/8] DynamicTablesPkg: IORT set reference to Id array only if present Sami Mujawar
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

Add error handling when the IdMappingCount is not zero and the
IdMappingToken is NULL.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
     - No changes since v3. Resending with v4 series.              [SAMI]
    v3:
     - New patch in this series. Moves error handling code for     [SAMI]
       IdMappingToken from patch v2 6/8 and v2 8/8 into this
       patch.
       Ref: https://edk2.groups.io/g/devel/topic/83600728#76662
            https://edk2.groups.io/g/devel/topic/83600726#76661

 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 82 ++++++++++++++++----
 1 file changed, 66 insertions(+), 16 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index 0f13c32b838bf4fe42b53a1e9c3ce817d74681fb..daf9ff00c3deab4005814bbfcf1650469d1e7d92 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -1,7 +1,7 @@
 /** @file
   IORT Table Generator
 
-  Copyright (c) 2017 - 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -905,9 +905,19 @@ AddNamedComponentNodes (
       return Status;
     }
 
-    if ((NodeList->IdMappingCount > 0) &&
-        (NodeList->IdMappingToken != CM_NULL_TOKEN))
-    {
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
       // Ids for Named Component
       IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)NcNode +
                                                           NcNode->Node.IdReference);
@@ -1011,9 +1021,19 @@ AddRootComplexNodes (
     RcNode->Reserved1[1]      = EFI_ACPI_RESERVED_BYTE;
     RcNode->Reserved1[2]      = EFI_ACPI_RESERVED_BYTE;
 
-    if ((NodeList->IdMappingCount > 0) &&
-        (NodeList->IdMappingToken != CM_NULL_TOKEN))
-    {
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
       // Ids for Root Complex
       IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)RcNode +
                                                           RcNode->Node.IdReference);
@@ -1242,9 +1262,19 @@ AddSmmuV1V2Nodes (
       }
     }
 
-    if ((NodeList->IdMappingCount > 0) &&
-        (NodeList->IdMappingToken != CM_NULL_TOKEN))
-    {
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
       // Ids for SMMU v1/v2 Node
       IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)SmmuNode +
                                                           SmmuNode->Node.IdReference);
@@ -1361,9 +1391,19 @@ AddSmmuV3Nodes (
       SmmuV3Node->DeviceIdMappingIndex = NodeList->DeviceIdMappingIndex;
     }
 
-    if ((NodeList->IdMappingCount > 0) &&
-        (NodeList->IdMappingToken != CM_NULL_TOKEN))
-    {
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
       // Ids for SMMUv3 node
       IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)SmmuV3Node +
                                                           SmmuV3Node->Node.IdReference);
@@ -1476,9 +1516,19 @@ AddPmcgNodes (
       return Status;
     }
 
-    if ((NodeList->IdMappingCount > 0) &&
-        (NodeList->IdMappingToken != CM_NULL_TOKEN))
-    {
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
       // Ids for PMCG node
       IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)PmcgNode +
                                                           PmcgNode->Node.IdReference);
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 5/8] DynamicTablesPkg: IORT set reference to Id array only if present
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
                   ` (3 preceding siblings ...)
  2022-07-06  9:56 ` [PATCH v4 4/8] DynamicTablesPkg: Handle error when IdMappingToken is NULL Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 6/8] DynamicTablesPkg: IORT set reference to interrupt array " Sami Mujawar
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

The IORT table generator is setting up a reference to ID array for
nodes even when the ID Mapping count is zero. This is not an issue as an
OS would only access the ID Reference if the ID mapping count is not zero.

However, it would be good to set the reference to ID array to zero when
the ID Mapping count is zero rather than populating it with an incorrect
value.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---

Notes:
    v4:
     - No code change since v1. Re-sending with v4 series.    [SAMI]
    
    v3:
     - No code change since v1. Re-sending with v3 series.    [SAMI]
    
    v2:
     - No code change since v1. Re-sending with v2 series.    [SAMI]

 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 27 +++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index daf9ff00c3deab4005814bbfcf1650469d1e7d92..a4dd3d4a895e0a1ae305c937d9a413665fb8e171 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -876,9 +876,9 @@ AddNamedComponentNodes (
     NcNode->Node.NumIdMappings = NodeList->IdMappingCount;
 
     ObjectNameLength         = AsciiStrLen (NodeList->ObjectName) + 1;
-    NcNode->Node.IdReference =
-      (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
-               (ALIGN_VALUE (ObjectNameLength, 4)));
+    NcNode->Node.IdReference = (NodeList->IdMappingCount == 0) ?
+                               0 : ((UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
+                                             (ALIGN_VALUE (ObjectNameLength, 4))));
 
     // Named Component specific data
     NcNode->Flags             = NodeList->Flags;
@@ -1007,7 +1007,8 @@ AddRootComplexNodes (
     RcNode->Node.Revision      = 1;
     RcNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     RcNode->Node.NumIdMappings = NodeList->IdMappingCount;
-    RcNode->Node.IdReference   = sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE);
+    RcNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
+                                 0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE);
 
     // Root Complex specific data
     RcNode->CacheCoherent     = NodeList->CacheCoherent;
@@ -1188,11 +1189,12 @@ AddSmmuV1V2Nodes (
     SmmuNode->Node.Revision      = 0;
     SmmuNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     SmmuNode->Node.NumIdMappings = NodeList->IdMappingCount;
-    SmmuNode->Node.IdReference   = sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
-                                   (NodeList->ContextInterruptCount *
-                                    sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
-                                   (NodeList->PmuInterruptCount *
-                                    sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT));
+    SmmuNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
+                                   0 : (sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
+                                        (NodeList->ContextInterruptCount *
+                                         sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
+                                        (NodeList->PmuInterruptCount *
+                                         sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)));
 
     // SMMU v1/v2 specific data
     SmmuNode->Base  = NodeList->BaseAddress;
@@ -1360,8 +1362,8 @@ AddSmmuV3Nodes (
     SmmuV3Node->Node.Revision      = 2;
     SmmuV3Node->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     SmmuV3Node->Node.NumIdMappings = NodeList->IdMappingCount;
-    SmmuV3Node->Node.IdReference   =
-      sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE);
+    SmmuV3Node->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
+                                     0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE);
 
     // SMMUv3 specific data
     SmmuV3Node->Base         = NodeList->BaseAddress;
@@ -1491,7 +1493,8 @@ AddPmcgNodes (
     PmcgNode->Node.Revision      = 1;
     PmcgNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     PmcgNode->Node.NumIdMappings = NodeList->IdMappingCount;
-    PmcgNode->Node.IdReference   = sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE);
+    PmcgNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
+                                   0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE);
 
     // PMCG specific data
     PmcgNode->Base                  = NodeList->BaseAddress;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 6/8] DynamicTablesPkg: IORT set reference to interrupt array if present
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
                   ` (4 preceding siblings ...)
  2022-07-06  9:56 ` [PATCH v4 5/8] DynamicTablesPkg: IORT set reference to Id array only if present Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH v4 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.d Sami Mujawar
  2022-07-06  9:56 ` [PATCH 8/8] DynamicTablesPkg: IORT generator updates for Rev E.d spec Sami Mujawar
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

The IORT generator is populating the reference field for Context and
PMU interrupts even if their count is zero.

Update the IORT generator to set the references only if the interrupt
count is not 0. Also add checks to ensure a valid reference token has
been provided.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---

Notes:
    v4:
     - Minor reordering of code to group initialisation and    [SAMI]
       checks for the number of Context and PMU interrupts.
    
    v3:
      - Move error handling for IdMappingToken.                [PIERRE]
      - Moved error handling for IdMappingToken in a separate  [SAMI]
        patch in v3 series.
        Ref: https://edk2.groups.io/g/devel/topic/83600728#76662
    
    v2:
      - No code change since v1. Re-sending with v2 series.    [SAMI]

 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 87 +++++++++++++-------
 1 file changed, 57 insertions(+), 30 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index a4dd3d4a895e0a1ae305c937d9a413665fb8e171..abef9e5a7f90a38e1d697227d3cd2c110db364a4 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -1164,6 +1164,7 @@ AddSmmuV1V2Nodes (
   EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT  *ContextInterruptArray;
   EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT  *PmuInterruptArray;
   UINT64                              NodeLength;
+  UINT32                              Offset;
 
   ASSERT (Iort != NULL);
 
@@ -1206,48 +1207,74 @@ AddSmmuV1V2Nodes (
     SmmuNode->GlobalInterruptArrayRef =
       OFFSET_OF (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE, SMMU_NSgIrpt);
 
+    Offset = sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE);
     // Context Interrupt
-    SmmuNode->NumContextInterrupts     = NodeList->ContextInterruptCount;
-    SmmuNode->ContextInterruptArrayRef =
-      sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE);
-    ContextInterruptArray =
-      (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT *)((UINT8 *)SmmuNode +
-                                             sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE));
+    SmmuNode->NumContextInterrupts = NodeList->ContextInterruptCount;
+    if (NodeList->ContextInterruptCount != 0) {
+      SmmuNode->ContextInterruptArrayRef = Offset;
+      ContextInterruptArray              =
+        (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT *)((UINT8 *)SmmuNode + Offset);
+      Offset += (NodeList->ContextInterruptCount *
+                 sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT));
+    }
 
     // PMU Interrupt
-    SmmuNode->NumPmuInterrupts     = NodeList->PmuInterruptCount;
-    SmmuNode->PmuInterruptArrayRef = SmmuNode->ContextInterruptArrayRef +
-                                     (NodeList->ContextInterruptCount *
-                                      sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT));
-    PmuInterruptArray =
-      (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT *)((UINT8 *)SmmuNode +
-                                             SmmuNode->PmuInterruptArrayRef);
+    SmmuNode->NumPmuInterrupts = NodeList->PmuInterruptCount;
+    if (NodeList->PmuInterruptCount != 0) {
+      SmmuNode->PmuInterruptArrayRef = Offset;
+      PmuInterruptArray              =
+        (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT *)((UINT8 *)SmmuNode + Offset);
+    }
 
     SmmuNode->SMMU_NSgIrpt         = NodeList->SMMU_NSgIrpt;
     SmmuNode->SMMU_NSgIrptFlags    = NodeList->SMMU_NSgIrptFlags;
     SmmuNode->SMMU_NSgCfgIrpt      = NodeList->SMMU_NSgCfgIrpt;
     SmmuNode->SMMU_NSgCfgIrptFlags = NodeList->SMMU_NSgCfgIrptFlags;
 
-    // Add Context Interrupt Array
-    Status = AddSmmuInterruptArray (
-               CfgMgrProtocol,
-               ContextInterruptArray,
-               SmmuNode->NumContextInterrupts,
-               NodeList->ContextInterruptToken
-               );
-    if (EFI_ERROR (Status)) {
-      DEBUG ((
-        DEBUG_ERROR,
-        "ERROR: IORT: Failed to Context Interrupt Array. Status = %r\n",
-        Status
-        ));
-      return Status;
+    if (NodeList->ContextInterruptCount != 0) {
+      if (NodeList->ContextInterruptToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Context Interrupt token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->ContextInterruptToken,
+          Status
+          ));
+        return Status;
+      }
+
+      // Add Context Interrupt Array
+      Status = AddSmmuInterruptArray (
+                 CfgMgrProtocol,
+                 ContextInterruptArray,
+                 SmmuNode->NumContextInterrupts,
+                 NodeList->ContextInterruptToken
+                 );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Failed to Context Interrupt Array. Status = %r\n",
+          Status
+          ));
+        return Status;
+      }
     }
 
     // Add PMU Interrupt Array
-    if ((SmmuNode->NumPmuInterrupts > 0) &&
-        (NodeList->PmuInterruptToken != CM_NULL_TOKEN))
-    {
+    if (SmmuNode->NumPmuInterrupts != 0) {
+      if (NodeList->PmuInterruptToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid PMU Interrupt token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->PmuInterruptToken,
+          Status
+          ));
+        return Status;
+      }
+
       Status = AddSmmuInterruptArray (
                  CfgMgrProtocol,
                  PmuInterruptArray,
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.d
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
                   ` (5 preceding siblings ...)
  2022-07-06  9:56 ` [PATCH v4 6/8] DynamicTablesPkg: IORT set reference to interrupt array " Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  2022-07-06  9:56 ` [PATCH 8/8] DynamicTablesPkg: IORT generator updates for Rev E.d spec Sami Mujawar
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, steven.price, Lorenzo.Pieralisi, nd

Bugzilla: 3458 - Add support IORT Rev E.d specification updates
          (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

The IO Remapping Table, Platform Design Document, Revision E.d,
    Feb 2022 (https://developer.arm.com/documentation/den0049/)
    introduces the following updates, collectively including the
    updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
     - increments the IORT table revision to 5.
     - updates the node definition to add an 'Identifier' field.
     - adds definition of node type 6 - Reserved Memory Range node.
     - adds definition for Memory Range Descriptors.
     - adds flag to indicate PRI support for root complexes.
     - adds flag to indicate if the root complex supports forwarding
       of PASID information on translated transactions to the SMMU.
     - adds flag to indicate if the root complex supports PASID.
     - adds flags to define access privilege and attributes for the
       memory ranges.

Therefore, update the Arm namespace objects to:
  - add Identifier field to IORT nodes.
  - introduce enums to represent RMR nodes and Memory Range
    descriptors.
  - add definition of node type 6 - Reserved Memory Range node.
  - add definition for Memory Range Descriptors.
  - add PASID capabilities and flags field to Root Complex node.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
    - Update ArmNameSpaceObjects to support IORT specification        [SAMI]
      revision E.d.
    - Add PASID capabilities and flags field to Root Complex node.    [SAMI]
    - Add flags to define access privilege and attributes for the     [SAMI]
      memory ranges.
    - Update DynamicTablesPkg.ci.yaml to add PASID to the ignore      [SAMI]
      list for the spell checker.
    
    v3:
     - Move Identifier field before Flags field in            [PIERRE]
       CM_ARM_RMR_NODE.
     - Add description for CM_ARM_MEMORY_RANGE_DESCRIPTOR     [PIERRE]
       field.
     - Updated based on review feedback.                      [SAMI]
       Ref: https://edk2.groups.io/g/devel/topic/83600723#76659
    
    v2:
     - No code change since v1. Re-sending with v2 series.    [SAMI]

 DynamicTablesPkg/DynamicTablesPkg.ci.yaml      |  1 +
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 66 +++++++++++++++++++-
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
index bfa282926e48c79ea748b12dee19a322197eaed1..5addf8626841fe35dd0d499a277cb7308787fee0 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
+++ b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
@@ -108,6 +108,7 @@
            "lgreater",
            "lless",
            "MPIDR",
+           "PASID",
            "PERIPHBASE",
            "phandle",
            "pytool",
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 91bef9bccd1978b0e396f423cff81e621b05e0ea..102e0f96beb22cc2b93c1525bef62cd4173774eb 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -61,6 +61,8 @@ typedef enum ArmObjectID {
   EArmObjLpiInfo,                      ///< 37 - Lpi Info
   EArmObjPciAddressMapInfo,            ///< 38 - Pci Address Map Info
   EArmObjPciInterruptMapInfo,          ///< 39 - Pci Interrupt Map Info
+  EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
+  EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -477,6 +479,9 @@ typedef struct CmArmItsGroupNode {
   UINT32             ItsIdCount;
   /// Reference token for the ITS identifier array
   CM_OBJECT_TOKEN    ItsIdToken;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_ITS_GROUP_NODE;
 
 /** A structure that describes the
@@ -509,6 +514,9 @@ typedef struct CmArmNamedComponentNode {
       the entry in the namespace for this object.
   */
   CHAR8              *ObjectName;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_NAMED_COMPONENT_NODE;
 
 /** A structure that describes the
@@ -537,6 +545,13 @@ typedef struct CmArmRootComplexNode {
   UINT32             PciSegmentNumber;
   /// Memory address size limit
   UINT8              MemoryAddressSize;
+  /// PASID capabilities
+  UINT16             PasidCapabilities;
+  /// Flags
+  UINT32             Flags;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_ROOT_COMPLEX_NODE;
 
 /** A structure that describes the
@@ -579,6 +594,9 @@ typedef struct CmArmSmmuV1SmmuV2Node {
   UINT32             SMMU_NSgCfgIrpt;
   /// SMMU_NSgCfgIrpt interrupt flags
   UINT32             SMMU_NSgCfgIrptFlags;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_SMMUV1_SMMUV2_NODE;
 
 /** A structure that describes the
@@ -615,6 +633,9 @@ typedef struct CmArmSmmuV3Node {
   UINT32             ProximityDomain;
   /// Index into the array of ID mapping
   UINT32             DeviceIdMappingIndex;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_SMMUV3_NODE;
 
 /** A structure that describes the
@@ -639,6 +660,9 @@ typedef struct CmArmPmcgNode {
 
   /// Reference token for the IORT node associated with this node
   CM_OBJECT_TOKEN    ReferenceToken;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
 } CM_ARM_PMCG_NODE;
 
 /** A structure that describes the
@@ -1006,6 +1030,46 @@ typedef struct CmArmPciInterruptMapInfo {
   CM_ARM_GENERIC_INTERRUPT    IntcInterrupt;
 } CM_ARM_PCI_INTERRUPT_MAP_INFO;
 
+/** A structure that describes the
+    RMR node for the Platform.
+
+    ID: EArmObjRmr
+*/
+typedef struct CmArmRmrNode {
+  /// An unique token used to identify this object
+  CM_OBJECT_TOKEN    Token;
+  /// Number of ID mappings
+  UINT32             IdMappingCount;
+  /// Reference token for the ID mapping array
+  CM_OBJECT_TOKEN    IdMappingToken;
+
+  /// Unique identifier for this node.
+  UINT32             Identifier;
+
+  /// Reserved Memory Range flags.
+  UINT32             Flags;
+
+  /// Memory range descriptor count.
+  UINT32             MemRangeDescCount;
+  /// Reference token for the Memory Range descriptor array
+  CM_OBJECT_TOKEN    MemRangeDescToken;
+} CM_ARM_RMR_NODE;
+
+/** A structure that describes the
+    Memory Range descriptor.
+
+    ID: EArmObjMemoryRangeDescriptor
+*/
+typedef struct CmArmRmrDescriptor {
+  /// Base address of Reserved Memory Range,
+  /// aligned to a page size of 64K.
+  UINT64    BaseAddress;
+
+  /// Length of the Reserved Memory range.
+  /// Must be a multiple of the page size of 64K.
+  UINT64    Length;
+} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
+
 #pragma pack()
 
 #endif // ARM_NAMESPACE_OBJECTS_H_
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] DynamicTablesPkg: IORT generator updates for Rev E.d spec
  2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
                   ` (6 preceding siblings ...)
  2022-07-06  9:56 ` [PATCH v4 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.d Sami Mujawar
@ 2022-07-06  9:56 ` Sami Mujawar
  7 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06  9:56 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, steven.price, Lorenzo.Pieralisi, nd

Bugzilla: 3458 - Add support IORT Rev E.d specification updates
          (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

The IO Remapping Table, Platform Design Document, Revision E.d,
Feb 2022 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
 - increments the IORT table revision to 5.
 - updates the node definition to add an 'Identifier' field.
 - adds definition of node type 6 - Reserved Memory Range node.
 - adds definition for Memory Range Descriptors.
 - adds flag to indicate PRI support for root complexes.
 - adds flag to indicate if the root complex supports forwarding
   of PASID information on translated transactions to the SMMU.
 - adds flag to indicate if the root complex supports PASID.
 - adds flags to define access privilege and attributes for the
   memory ranges.

Therefore, update the IORT generator to:
  - increment IORT table revision count to 5.
  - populate Identifier filed if revision is greater than 4.
  - add support to populate Reserved Memory Range nodes and
    the Memory range descriptors.
  - add validation to check that the Identifier field is
    unique.
  - Populate the PASID capabilities and Flags field of the
    Root complex node.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
     - Update IORT generator to support spec revision E.d        [SAMI]
     - Populate the PASID capabilities and Flags field of the    [SAMI]
       Root complex node.
    
    v3:
     - Move error handling for IdMappingToken.                   [PIERRE]
     - Moved error handling for IdMappingToken in a separate     [SAMI]
       patch in v3 series.
       Ref: https://edk2.groups.io/g/devel/topic/83600726#76661
    
    v2:
     - The macro EFI_ACPI_IO_REMAPPING_TABLE_REVISION was set to [SAMI]
       Rev 0 as setting to Rev 3 will break existing platforms.
       Therefore, set the max supported IORT generator revision
       to 3.

 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 630 ++++++++++++++++++--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h |   5 +-
 2 files changed, 583 insertions(+), 52 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index abef9e5a7f90a38e1d697227d3cd2c110db364a4..a3700cd31b72151ed287568277971e7ed8c57295 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -5,8 +5,8 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-  - IO Remapping Table, Platform Design Document,
-    Document number: ARM DEN 0049D, Issue D, March 2018
+  - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
+    (https://developer.arm.com/documentation/den0049/)
 
 **/
 
@@ -37,9 +37,11 @@ Requirements:
   - EArmObjSmmuV1SmmuV2
   - EArmObjSmmuV3
   - EArmObjPmcg
+  - EArmObjRmr
   - EArmObjGicItsIdentifierArray
   - EArmObjIdMappingArray
-  - EArmObjGicItsIdentifierArray
+  - EArmObjSmmuInterruptArray
+  - EArmObjMemoryRangeDescriptor
 */
 
 /** This macro expands to a function that retrieves the ITS
@@ -96,6 +98,24 @@ GET_OBJECT_LIST (
   CM_ARM_PMCG_NODE
   );
 
+/** This macro expands to a function that retrieves the
+    RMR node information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjRmr,
+  CM_ARM_RMR_NODE
+  );
+
+/** This macro expands to a function that retrieves the
+    Memory Range Descriptor Array information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjMemoryRangeDescriptor,
+  CM_ARM_MEMORY_RANGE_DESCRIPTOR
+  );
+
 /** This macro expands to a function that retrieves the
     ITS Identifier Array information from the Configuration Manager.
 */
@@ -174,16 +194,19 @@ GetSizeofItsGroupNodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetItsGroupNodeSize (NodeList);
@@ -248,16 +271,19 @@ GetSizeofNamedComponentNodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetNamedComponentNodeSize (NodeList);
@@ -320,16 +346,19 @@ GetSizeofRootComplexNodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetRootComplexNodeSize (NodeList);
@@ -398,16 +427,19 @@ GetSizeofSmmuV1V2Nodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetSmmuV1V2NodeSize (NodeList);
@@ -470,16 +502,19 @@ GetSizeofSmmuV3Nodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetSmmuV3NodeSize (NodeList);
@@ -542,16 +577,19 @@ GetSizeofPmcgNodes (
 
   Size = 0;
   while (NodeCount-- != 0) {
-    (*NodeIndexer)->Token  = NodeList->Token;
-    (*NodeIndexer)->Object = (VOID *)NodeList;
-    (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Token      = NodeList->Token;
+    (*NodeIndexer)->Object     = (VOID *)NodeList;
+    (*NodeIndexer)->Offset     = (UINT32)(Size + NodeStartOffset);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
     DEBUG ((
       DEBUG_INFO,
-      "IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
       *NodeIndexer,
       (*NodeIndexer)->Token,
       (*NodeIndexer)->Object,
-      (*NodeIndexer)->Offset
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
       ));
 
     Size += GetPmcgNodeSize (NodeList);
@@ -562,6 +600,84 @@ GetSizeofPmcgNodes (
   return Size;
 }
 
+/** Returns the size of the RMR node.
+
+    @param [in]  Node    Pointer to RMR node.
+
+    @retval Size of the RMR node.
+**/
+STATIC
+UINT32
+GetRmrNodeSize (
+  IN  CONST CM_ARM_RMR_NODE  *Node
+  )
+{
+  ASSERT (Node != NULL);
+
+  /* Size of RMR node +
+     Size of ID mapping array +
+     Size of Memory Range Descriptor array
+  */
+  return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE) +
+                  (Node->IdMappingCount *
+                   sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
+                  (Node->MemRangeDescCount *
+                   sizeof (EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC)));
+}
+
+/** Returns the total size required for the RMR nodes 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
+                                      IORT where this node group starts.
+    @param [in]       NodeList        Pointer to RMR node list.
+    @param [in]       NodeCount       Count of the RMR nodes.
+    @param [in, out]  NodeIndexer     Pointer to the next Node Indexer.
+
+    @retval Total size of the RMR nodes.
+**/
+STATIC
+UINT64
+GetSizeofRmrNodes (
+  IN      CONST UINT32                     NodeStartOffset,
+  IN      CONST CM_ARM_RMR_NODE            *NodeList,
+  IN            UINT32                     NodeCount,
+  IN OUT        IORT_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);
+    (*NodeIndexer)->Identifier = NodeList->Identifier;
+    DEBUG ((
+      DEBUG_INFO,
+      "IORT: Node Indexer = %p, Token = %p, Object = %p,"
+      " Offset = 0x%x, Identifier = 0x%x\n",
+      *NodeIndexer,
+      (*NodeIndexer)->Token,
+      (*NodeIndexer)->Object,
+      (*NodeIndexer)->Offset,
+      (*NodeIndexer)->Identifier
+      ));
+
+    Size += GetRmrNodeSize (NodeList);
+    (*NodeIndexer)++;
+    NodeList++;
+  }
+
+  return Size;
+}
+
 /** Returns the offset of the Node referenced by the Token.
 
     @param [in]  NodeIndexer  Pointer to node indexer array.
@@ -713,6 +829,7 @@ AddIdMappingArray (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the ITS Group
                                      Nodes.
@@ -729,6 +846,7 @@ EFI_STATUS
 AddItsGroupNodes (
   IN  CONST ACPI_TABLE_GENERATOR                  *CONST  This,
   IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN  CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
   IN  CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE               *Iort,
   IN  CONST UINT32                                        NodesStartOffset,
   IN  CONST CM_ARM_ITS_GROUP_NODE                         *NodeList,
@@ -765,11 +883,17 @@ AddItsGroupNodes (
     // Populate the node header
     ItsGroupNode->Node.Type          = EFI_ACPI_IORT_TYPE_ITS_GROUP;
     ItsGroupNode->Node.Length        = (UINT16)NodeLength;
-    ItsGroupNode->Node.Revision      = 0;
-    ItsGroupNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     ItsGroupNode->Node.NumIdMappings = 0;
     ItsGroupNode->Node.IdReference   = 0;
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      ItsGroupNode->Node.Revision   = 0;
+      ItsGroupNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      ItsGroupNode->Node.Revision   = 1;
+      ItsGroupNode->Node.Identifier = NodeList->Identifier;
+    }
+
     // IORT specific data
     ItsGroupNode->NumItsIdentifiers = NodeList->ItsIdCount;
     ItsIds                          = (UINT32 *)((UINT8 *)ItsGroupNode +
@@ -820,6 +944,7 @@ AddItsGroupNodes (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the Named
                                      Component Nodes.
@@ -836,6 +961,7 @@ EFI_STATUS
 AddNamedComponentNodes (
   IN      CONST ACPI_TABLE_GENERATOR                   *CONST  This,
   IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL   *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO             *CONST  AcpiTableInfo,
   IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE                *Iort,
   IN      CONST UINT32                                         NodesStartOffset,
   IN      CONST CM_ARM_NAMED_COMPONENT_NODE                    *NodeList,
@@ -871,10 +997,16 @@ AddNamedComponentNodes (
     // Populate the node header
     NcNode->Node.Type          = EFI_ACPI_IORT_TYPE_NAMED_COMP;
     NcNode->Node.Length        = (UINT16)NodeLength;
-    NcNode->Node.Revision      = 2;
-    NcNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     NcNode->Node.NumIdMappings = NodeList->IdMappingCount;
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      NcNode->Node.Revision   = 2;
+      NcNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      NcNode->Node.Revision   = 4;
+      NcNode->Node.Identifier = NodeList->Identifier;
+    }
+
     ObjectNameLength         = AsciiStrLen (NodeList->ObjectName) + 1;
     NcNode->Node.IdReference = (NodeList->IdMappingCount == 0) ?
                                0 : ((UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
@@ -955,6 +1087,7 @@ AddNamedComponentNodes (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the Root Complex
                                      Nodes.
@@ -971,6 +1104,7 @@ EFI_STATUS
 AddRootComplexNodes (
   IN      CONST ACPI_TABLE_GENERATOR                   *CONST  This,
   IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL   *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO             *CONST  AcpiTableInfo,
   IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE                *Iort,
   IN      CONST UINT32                                         NodesStartOffset,
   IN      CONST CM_ARM_ROOT_COMPLEX_NODE                       *NodeList,
@@ -1004,12 +1138,20 @@ AddRootComplexNodes (
     // Populate the node header
     RcNode->Node.Type          = EFI_ACPI_IORT_TYPE_ROOT_COMPLEX;
     RcNode->Node.Length        = (UINT16)NodeLength;
-    RcNode->Node.Revision      = 1;
-    RcNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     RcNode->Node.NumIdMappings = NodeList->IdMappingCount;
     RcNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
                                  0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE);
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      RcNode->Node.Revision   = 1;
+      RcNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      RcNode->Node.Revision     = 4;
+      RcNode->Node.Identifier   = NodeList->Identifier;
+      RcNode->PasidCapabilities = NodeList->PasidCapabilities;
+      RcNode->Flags             = NodeList->Flags;
+    }
+
     // Root Complex specific data
     RcNode->CacheCoherent     = NodeList->CacheCoherent;
     RcNode->AllocationHints   = NodeList->AllocationHints;
@@ -1019,8 +1161,6 @@ AddRootComplexNodes (
     RcNode->PciSegmentNumber  = NodeList->PciSegmentNumber;
     RcNode->MemoryAddressSize = NodeList->MemoryAddressSize;
     RcNode->Reserved1[0]      = EFI_ACPI_RESERVED_BYTE;
-    RcNode->Reserved1[1]      = EFI_ACPI_RESERVED_BYTE;
-    RcNode->Reserved1[2]      = EFI_ACPI_RESERVED_BYTE;
 
     if (NodeList->IdMappingCount > 0) {
       if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
@@ -1135,6 +1275,7 @@ AddSmmuInterruptArray (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the SMMU v1/v2
                                      Nodes.
@@ -1151,6 +1292,7 @@ EFI_STATUS
 AddSmmuV1V2Nodes (
   IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
   IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
   IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE               *Iort,
   IN      CONST UINT32                                        NodesStartOffset,
   IN      CONST CM_ARM_SMMUV1_SMMUV2_NODE                     *NodeList,
@@ -1187,8 +1329,6 @@ AddSmmuV1V2Nodes (
     // Populate the node header
     SmmuNode->Node.Type          = EFI_ACPI_IORT_TYPE_SMMUv1v2;
     SmmuNode->Node.Length        = (UINT16)NodeLength;
-    SmmuNode->Node.Revision      = 0;
-    SmmuNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     SmmuNode->Node.NumIdMappings = NodeList->IdMappingCount;
     SmmuNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
                                    0 : (sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
@@ -1197,6 +1337,14 @@ AddSmmuV1V2Nodes (
                                         (NodeList->PmuInterruptCount *
                                          sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)));
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      SmmuNode->Node.Revision   = 1;
+      SmmuNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      SmmuNode->Node.Revision   = 3;
+      SmmuNode->Node.Identifier = NodeList->Identifier;
+    }
+
     // SMMU v1/v2 specific data
     SmmuNode->Base  = NodeList->BaseAddress;
     SmmuNode->Span  = NodeList->Span;
@@ -1340,6 +1488,7 @@ AddSmmuV1V2Nodes (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the SMMUv3 Nodes.
     @param [in]     NodeList         Pointer to an array of SMMUv3 Node Objects.
@@ -1354,6 +1503,7 @@ EFI_STATUS
 AddSmmuV3Nodes (
   IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
   IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
   IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE               *Iort,
   IN      CONST UINT32                                        NodesStartOffset,
   IN      CONST CM_ARM_SMMUV3_NODE                            *NodeList,
@@ -1386,12 +1536,18 @@ AddSmmuV3Nodes (
     // Populate the node header
     SmmuV3Node->Node.Type          = EFI_ACPI_IORT_TYPE_SMMUv3;
     SmmuV3Node->Node.Length        = (UINT16)NodeLength;
-    SmmuV3Node->Node.Revision      = 2;
-    SmmuV3Node->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     SmmuV3Node->Node.NumIdMappings = NodeList->IdMappingCount;
     SmmuV3Node->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
                                      0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE);
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      SmmuV3Node->Node.Revision   = 2;
+      SmmuV3Node->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      SmmuV3Node->Node.Revision   = 4;
+      SmmuV3Node->Node.Identifier = NodeList->Identifier;
+    }
+
     // SMMUv3 specific data
     SmmuV3Node->Base         = NodeList->BaseAddress;
     SmmuV3Node->Flags        = NodeList->Flags;
@@ -1469,6 +1625,7 @@ AddSmmuV3Nodes (
     @param [in]     This             Pointer to the table Generator.
     @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
                                      Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
     @param [in]     Iort             Pointer to IORT table structure.
     @param [in]     NodesStartOffset Offset for the start of the PMCG Nodes.
     @param [in]     NodeList         Pointer to an array of PMCG Node Objects.
@@ -1483,6 +1640,7 @@ EFI_STATUS
 AddPmcgNodes (
   IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
   IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
   IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE               *Iort,
   IN      CONST UINT32                                        NodesStartOffset,
   IN      CONST CM_ARM_PMCG_NODE                              *NodeList,
@@ -1517,12 +1675,18 @@ AddPmcgNodes (
     // Populate the node header
     PmcgNode->Node.Type          = EFI_ACPI_IORT_TYPE_PMCG;
     PmcgNode->Node.Length        = (UINT16)NodeLength;
-    PmcgNode->Node.Revision      = 1;
-    PmcgNode->Node.Reserved      = EFI_ACPI_RESERVED_DWORD;
     PmcgNode->Node.NumIdMappings = NodeList->IdMappingCount;
     PmcgNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
                                    0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE);
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+      PmcgNode->Node.Revision   = 1;
+      PmcgNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      PmcgNode->Node.Revision   = 2;
+      PmcgNode->Node.Identifier = NodeList->Identifier;
+    }
+
     // PMCG specific data
     PmcgNode->Base                  = NodeList->BaseAddress;
     PmcgNode->OverflowInterruptGsiv = NodeList->OverflowInterrupt;
@@ -1589,6 +1753,274 @@ AddPmcgNodes (
   return EFI_SUCCESS;
 }
 
+/** Update the Memory Range Descriptor Array.
+
+    This function retrieves the Memory Range Descriptor objects referenced by
+    MemRangeDescToken and updates the Memory Range Descriptor array.
+
+    @param [in]     This             Pointer to the table Generator.
+    @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
+                                     Protocol Interface.
+    @param [in]     DescArray        Pointer to an array of Memory Range
+                                     Descriptors.
+    @param [in]     DescCount        Number of Id Descriptors.
+    @param [in]     DescToken        Reference Token for retrieving the
+                                     Memory Range Descriptor Array.
+
+    @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
+AddMemRangeDescArray (
+  IN  CONST ACPI_TABLE_GENERATOR                      *CONST  This,
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL      *CONST  CfgMgrProtocol,
+  IN        EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC          *DescArray,
+  IN        UINT32                                            DescCount,
+  IN  CONST CM_OBJECT_TOKEN                                   DescToken
+  )
+{
+  EFI_STATUS                      Status;
+  CM_ARM_MEMORY_RANGE_DESCRIPTOR  *MemRangeDesc;
+  UINT32                          MemRangeDescCount;
+
+  ASSERT (DescArray != NULL);
+
+  // Get the Id Mapping Array
+  Status = GetEArmObjMemoryRangeDescriptor (
+             CfgMgrProtocol,
+             DescToken,
+             &MemRangeDesc,
+             &MemRangeDescCount
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: IORT: Failed to get Memory Range Descriptor array. Status = %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  if (MemRangeDescCount < DescCount) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: IORT: Failed to get the required number of Memory"
+      " Range Descriptors.\n"
+      ));
+    return EFI_NOT_FOUND;
+  }
+
+  // Populate the Memory Range Descriptor array
+  while (DescCount-- != 0) {
+    DescArray->Base     = MemRangeDesc->BaseAddress;
+    DescArray->Length   = MemRangeDesc->Length;
+    DescArray->Reserved = EFI_ACPI_RESERVED_DWORD;
+
+    DescArray++;
+    MemRangeDesc++;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/** Update the RMR Node Information.
+
+    This function updates the RMR node information in the IORT table.
+
+    @param [in]     This             Pointer to the table Generator.
+    @param [in]     CfgMgrProtocol   Pointer to the Configuration Manager
+                                     Protocol Interface.
+    @param [in]     AcpiTableInfo    Pointer to the ACPI table info structure.
+    @param [in]     Iort             Pointer to IORT table structure.
+    @param [in]     NodesStartOffset Offset for the start of the PMCG Nodes.
+    @param [in]     NodeList         Pointer to an array of PMCG Node Objects.
+    @param [in]     NodeCount        Number of PMCG 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
+AddRmrNodes (
+  IN      CONST ACPI_TABLE_GENERATOR                  *CONST  This,
+  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN      CONST CM_STD_OBJ_ACPI_TABLE_INFO            *CONST  AcpiTableInfo,
+  IN      CONST EFI_ACPI_6_0_IO_REMAPPING_TABLE               *Iort,
+  IN      CONST UINT32                                        NodesStartOffset,
+  IN      CONST CM_ARM_RMR_NODE                               *NodeList,
+  IN            UINT32                                        NodeCount
+  )
+{
+  EFI_STATUS                                Status;
+  EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE        *RmrNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        *IdMapArray;
+  EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC  *MemRangeDescArray;
+  UINT64                                    NodeLength;
+
+  ASSERT (Iort != NULL);
+
+  RmrNode = (EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE *)((UINT8 *)Iort +
+                                                   NodesStartOffset);
+
+  while (NodeCount-- != 0) {
+    NodeLength = GetRmrNodeSize (NodeList);
+    if (NodeLength > MAX_UINT16) {
+      Status = EFI_INVALID_PARAMETER;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: RMR Node length 0x%lx > MAX_UINT16. Status = %r\n",
+        NodeLength,
+        Status
+        ));
+      return Status;
+    }
+
+    if (NodeList->MemRangeDescCount == 0) {
+      Status = EFI_INVALID_PARAMETER;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Memory Range Desc count = %d. Status = %r\n",
+        NodeList->MemRangeDescCount,
+        Status
+        ));
+      return Status;
+    }
+
+    if (NodeList->MemRangeDescToken == CM_NULL_TOKEN) {
+      Status = EFI_INVALID_PARAMETER;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Invalid Memory Range Descriptor token,"
+        " Token = 0x%x. Status = %r\n",
+        NodeList->MemRangeDescToken,
+        Status
+        ));
+      return Status;
+    }
+
+    // Populate the node header
+    RmrNode->Node.Type          = EFI_ACPI_IORT_TYPE_RMR;
+    RmrNode->Node.Length        = (UINT16)NodeLength;
+    RmrNode->Node.Revision      = 3;
+    RmrNode->Node.Identifier    = NodeList->Identifier;
+    RmrNode->Node.NumIdMappings = NodeList->IdMappingCount;
+    RmrNode->Node.IdReference   = (NodeList->IdMappingCount == 0) ?
+                                  0 : sizeof (EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE);
+
+    // RMR specific data
+    RmrNode->Flags           = NodeList->Flags;
+    RmrNode->NumMemRangeDesc = NodeList->MemRangeDescCount;
+    RmrNode->MemRangeDescRef = (NodeList->MemRangeDescCount == 0) ?
+                               0 : (sizeof (EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE) +
+                                    (NodeList->IdMappingCount *
+                                     sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
+
+    if (NodeList->IdMappingCount > 0) {
+      if (NodeList->IdMappingToken == CM_NULL_TOKEN) {
+        Status = EFI_INVALID_PARAMETER;
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Invalid Id Mapping token,"
+          " Token = 0x%x, Status =%r\n",
+          NodeList->IdMappingToken,
+          Status
+          ));
+        return Status;
+      }
+
+      // Ids for RMR node
+      IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE *)((UINT8 *)RmrNode +
+                                                          RmrNode->Node.IdReference);
+
+      Status = AddIdMappingArray (
+                 This,
+                 CfgMgrProtocol,
+                 IdMapArray,
+                 NodeList->IdMappingCount,
+                 NodeList->IdMappingToken
+                 );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: Failed to add Id Mapping Array. Status = %r\n",
+          Status
+          ));
+        return Status;
+      }
+    }
+
+    // Memory Range Descriptors for RMR node
+    MemRangeDescArray = (EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC *)(
+                                                                     (UINT8 *)RmrNode +
+                                                                     RmrNode->MemRangeDescRef
+                                                                     );
+
+    Status = AddMemRangeDescArray (
+               This,
+               CfgMgrProtocol,
+               MemRangeDescArray,
+               NodeList->MemRangeDescCount,
+               NodeList->MemRangeDescToken
+               );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Failed to Memory Range Descriptor Array. Status = %r\n",
+        Status
+        ));
+      return Status;
+    }
+
+    // Next RMR Node
+    RmrNode = (EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE *)((UINT8 *)RmrNode +
+                                                     RmrNode->Node.Length);
+    NodeList++;
+  } // RMR Node
+
+  return EFI_SUCCESS;
+}
+
+/** Validates that the IORT nodes Identifier are unique.
+
+    @param [in]     NodeIndexer      Pointer to the Node Indexer.
+    @param [in]     NodeCount        Number of IORT Nodes.
+
+    @retval EFI_SUCCESS             Success.
+    @retval EFI_INVALID_PARAMETER   Identifier field not unique.
+**/
+STATIC
+EFI_STATUS
+ValidateNodeIdentifiers (
+  IN      CONST IORT_NODE_INDEXER                  *CONST  NodeIndexer,
+  IN            UINT32                                     NodeCount
+  )
+{
+  UINT32  IndexI;
+  UINT32  IndexJ;
+
+  for (IndexI = 0; IndexI < NodeCount; IndexI++) {
+    for (IndexJ = 0; IndexJ < NodeCount; IndexJ++) {
+      if ((IndexI != IndexJ) &&
+          (NodeIndexer[IndexI].Identifier == NodeIndexer[IndexJ].Identifier))
+      {
+        DEBUG ((
+          DEBUG_ERROR,
+          "ERROR: IORT: UID %d of Token %p matches with that of Token %p.\n",
+          NodeIndexer[IndexI].Identifier,
+          NodeIndexer[IndexI].Token,
+          NodeIndexer[IndexJ].Token
+          ));
+        return EFI_INVALID_PARAMETER;
+      }
+    }// IndexJ
+  } // IndexI
+
+  return EFI_SUCCESS;
+}
+
 /** Construct the IORT ACPI table.
 
     This function invokes the Configuration Manager protocol interface
@@ -1633,6 +2065,7 @@ BuildIortTable (
   UINT32  SmmuV1V2NodeCount;
   UINT32  SmmuV3NodeCount;
   UINT32  PmcgNodeCount;
+  UINT32  RmrNodeCount;
 
   UINT32  ItsGroupOffset;
   UINT32  NamedComponentOffset;
@@ -1640,6 +2073,7 @@ BuildIortTable (
   UINT32  SmmuV1V2Offset;
   UINT32  SmmuV3Offset;
   UINT32  PmcgOffset;
+  UINT32  RmrOffset;
 
   CM_ARM_ITS_GROUP_NODE        *ItsGroupNodeList;
   CM_ARM_NAMED_COMPONENT_NODE  *NamedComponentNodeList;
@@ -1647,6 +2081,7 @@ BuildIortTable (
   CM_ARM_SMMUV1_SMMUV2_NODE    *SmmuV1V2NodeList;
   CM_ARM_SMMUV3_NODE           *SmmuV3NodeList;
   CM_ARM_PMCG_NODE             *PmcgNodeList;
+  CM_ARM_RMR_NODE              *RmrNodeList;
 
   EFI_ACPI_6_0_IO_REMAPPING_TABLE  *Iort;
   IORT_NODE_INDEXER                *NodeIndexer;
@@ -1790,6 +2225,27 @@ BuildIortTable (
   // Add the PMCG node count
   IortNodeCount += PmcgNodeCount;
 
+  if (AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+    // Get the RMR node info
+    Status = GetEArmObjRmr (
+               CfgMgrProtocol,
+               CM_NULL_TOKEN,
+               &RmrNodeList,
+               &RmrNodeCount
+               );
+    if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Failed to get RMR Node Info. Status = %r\n",
+        Status
+        ));
+      goto error_handler;
+    }
+
+    // Add the RMR node count
+    IortNodeCount += RmrNodeCount;
+  }
+
   // Allocate Node Indexer array
   NodeIndexer = (IORT_NODE_INDEXER *)AllocateZeroPool (
                                        (sizeof (IORT_NODE_INDEXER) *
@@ -1999,6 +2455,39 @@ BuildIortTable (
       ));
   }
 
+  // RMR Nodes
+  if ((AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV5) &&
+      (RmrNodeCount > 0))
+  {
+    RmrOffset = (UINT32)TableSize;
+    // Size of RMR node list.
+    NodeSize = GetSizeofRmrNodes (
+                 RmrOffset,
+                 RmrNodeList,
+                 RmrNodeCount,
+                 &NodeIndexer
+                 );
+    if (NodeSize > MAX_UINT32) {
+      Status = EFI_INVALID_PARAMETER;
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Invalid Size of RMR Nodes. Status = %r\n",
+        Status
+        ));
+      goto error_handler;
+    }
+
+    TableSize += NodeSize;
+
+    DEBUG ((
+      DEBUG_INFO,
+      " RmrNodeCount = %d\n" \
+      " RmrOffset = %d\n",
+      RmrNodeCount,
+      RmrOffset
+      ));
+  }
+
   DEBUG ((
     DEBUG_INFO,
     "INFO: IORT:\n" \
@@ -2020,6 +2509,19 @@ BuildIortTable (
     goto error_handler;
   }
 
+  // Validate that the identifiers for the nodes are unique
+  if (AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV5) {
+    Status = ValidateNodeIdentifiers (Generator->NodeIndexer, IortNodeCount);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Node Identifier not unique. Status = %r\n",
+        Status
+        ));
+      goto error_handler;
+    }
+  }
+
   // Allocate the Buffer for IORT table
   *Table = (EFI_ACPI_DESCRIPTION_HEADER *)AllocateZeroPool (TableSize);
   if (*Table == NULL) {
@@ -2068,6 +2570,7 @@ BuildIortTable (
     Status = AddItsGroupNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                ItsGroupOffset,
                ItsGroupNodeList,
@@ -2087,6 +2590,7 @@ BuildIortTable (
     Status = AddNamedComponentNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                NamedComponentOffset,
                NamedComponentNodeList,
@@ -2106,6 +2610,7 @@ BuildIortTable (
     Status = AddRootComplexNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                RootComplexOffset,
                RootComplexNodeList,
@@ -2125,6 +2630,7 @@ BuildIortTable (
     Status = AddSmmuV1V2Nodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                SmmuV1V2Offset,
                SmmuV1V2NodeList,
@@ -2144,6 +2650,7 @@ BuildIortTable (
     Status = AddSmmuV3Nodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                SmmuV3Offset,
                SmmuV3NodeList,
@@ -2163,6 +2670,7 @@ BuildIortTable (
     Status = AddPmcgNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                PmcgOffset,
                PmcgNodeList,
@@ -2171,7 +2679,27 @@ BuildIortTable (
     if (EFI_ERROR (Status)) {
       DEBUG ((
         DEBUG_ERROR,
-        "ERROR: IORT: Failed to add SMMUv3 Node. Status = %r\n",
+        "ERROR: IORT: Failed to add PMCG Node. Status = %r\n",
+        Status
+        ));
+      goto error_handler;
+    }
+  }
+
+  if (RmrNodeCount > 0) {
+    Status = AddRmrNodes (
+               This,
+               CfgMgrProtocol,
+               AcpiTableInfo,
+               Iort,
+               RmrOffset,
+               RmrNodeList,
+               RmrNodeCount
+               );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: IORT: Failed to add RMR Node. Status = %r\n",
         Status
         ));
       goto error_handler;
@@ -2256,11 +2784,11 @@ ACPI_IORT_GENERATOR  IortGenerator = {
     // Generator Description
     L"ACPI.STD.IORT.GENERATOR",
     // ACPI Table Signature
-    EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE,
+    EFI_ACPI_6_3_IO_REMAPPING_TABLE_SIGNATURE,
     // ACPI Table Revision supported by this Generator
-    EFI_ACPI_IO_REMAPPING_TABLE_REVISION,
+    EFI_ACPI_IO_REMAPPING_TABLE_REV5,
     // Minimum supported ACPI Table Revision
-    EFI_ACPI_IO_REMAPPING_TABLE_REVISION,
+    EFI_ACPI_IO_REMAPPING_TABLE_REV0,
     // Creator ID
     TABLE_GENERATOR_CREATOR_ID_ARM,
     // Creator Revision
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h
index 61f6b6153f207b356ed26aabf8366c1cf632fd90..99d86b3d167b56ae4e0c22d292e6e33318e3c78f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -25,6 +25,9 @@ typedef struct IortNodeIndexer {
   VOID               *Object;
   /// Node offset from the start of the IORT table
   UINT32             Offset;
+
+  /// Unique identifier for the Node
+  UINT32             Identifier;
 } IORT_NODE_INDEXER;
 
 typedef struct AcpiIortGenerator {
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
@ 2022-07-06 15:44   ` Sami Mujawar
  2022-07-07  1:51     ` 回复: " gaoliming
  2022-07-07  6:05   ` Jon Nettleton
  1 sibling, 1 reply; 14+ messages in thread
From: Sami Mujawar @ 2022-07-06 15:44 UTC (permalink / raw)
  To: Sami Mujawar, devel

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On Wed, Jul 6, 2022 at 02:57 AM, Sami Mujawar wrote:

> 
> -#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION 0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV0 0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV5 0x5

Based on discussion at https://edk2.groups.io/g/devel/topic/patch_edk2_platforms_v2/92203690?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,92203690,previd%3D1657121904386162709,nextid%3D1657101454237486469&previd=1657121904386162709&nextid=1657101454237486469, the suggestion is to rename these macros to EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00 and EFI_ACPI_IO_REMAPPING_TABLE_REVISION_05 respectively.
I will wait for any other feedback on this series, before I send out the updated series reflecting the macro renaming.

Regards,

Sami Mujawar

[-- Attachment #2: Type: text/html, Size: 1109 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* 回复: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-06 15:44   ` [edk2-devel] " Sami Mujawar
@ 2022-07-07  1:51     ` gaoliming
  2022-07-07 10:31       ` Sami Mujawar
  0 siblings, 1 reply; 14+ messages in thread
From: gaoliming @ 2022-07-07  1:51 UTC (permalink / raw)
  To: devel, sami.mujawar

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

Sami:

 I suggest to keep EFI_ACPI_IO_REMAPPING_TABLE_REVISION. Its value can be EFI_ACPI_IO_REMAPPING_TABLE_REV5, because the structure has been updated, such as EFI_ACPI_6_0_IO_REMAPPING_RC_NODE. 

 

 Compared to the previous version, EFI_ACPI_6_0_IO_REMAPPING_RC_NODE struct is larger. Does this change bring any impact?

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sami Mujawar
发送时间: 2022年7月6日 23:44
收件人: Sami Mujawar <sami.mujawar@arm.com>; devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec

 

On Wed, Jul 6, 2022 at 02:57 AM, Sami Mujawar wrote:

-#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION 0x0
+#define EFI_ACPI_IO_REMAPPING_TABLE_REV0 0x0
+#define EFI_ACPI_IO_REMAPPING_TABLE_REV5 0x5

Based on discussion at https://edk2.groups.io/g/devel/topic/patch_edk2_platforms_v2/92203690?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,92203690,previd%3D1657121904386162709,nextid%3D1657101454237486469 <https://edk2.groups.io/g/devel/topic/patch_edk2_platforms_v2/92203690?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,92203690,previd%3D1657121904386162709,nextid%3D1657101454237486469&previd=1657121904386162709&nextid=1657101454237486469,> &previd=1657121904386162709&nextid=1657101454237486469, the suggestion is to rename these macros to EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00 and EFI_ACPI_IO_REMAPPING_TABLE_REVISION_05 respectively.
I will wait for any other feedback on this series, before I send out the updated series reflecting the macro renaming.

Regards,

Sami Mujawar 




[-- Attachment #2: Type: text/html, Size: 5474 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
  2022-07-06 15:44   ` [edk2-devel] " Sami Mujawar
@ 2022-07-07  6:05   ` Jon Nettleton
  2022-07-07 11:02     ` Sami Mujawar
  1 sibling, 1 reply; 14+ messages in thread
From: Jon Nettleton @ 2022-07-07  6:05 UTC (permalink / raw)
  To: edk2-devel-groups-io, Sami Mujawar
  Cc: Alexei.Fedorov, Ard Biesheuvel (TianoCore), quic_llindhol,
	pierre.gondois, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	Steven Price, Lorenzo Pieralisi, michael.d.kinney, Liming Gao,
	zhiguang.liu, ray.ni, zhichao.gao, nd

On Wed, Jul 6, 2022 at 11:57 AM Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> Bugzilla: 3458 - Add support IORT Rev E.d specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.d,
> Feb 2022 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
>   - increments the IORT table revision to 5.
>   - updates the node definition to add an 'Identifier' field.
>   - adds definition of node type 6 - Reserved Memory Range node.
>   - adds definition for Memory Range Descriptors.
>   - adds flag to indicate PRI support for root complexes.
>   - adds flag to indicate if the root complex supports forwarding
>     of PASID information on translated transactions to the SMMU.
>   - adds flag to indicate if the root complex supports PASID.
>   - adds flags to define access privilege and attributes for the
>     memory ranges.
>
> Therefore, update the IORT header file to reflect these changes,
> and also rename the EFI_ACPI_IO_REMAPPING_TABLE_REVISION macro to
> EFI_ACPI_IO_REMAPPING_TABLE_REV0.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
>     v4:
>       - Updated patch series to IORT specification revision E.d.     [SAMI]
>       - Add flag to indicate if the root complex supports PASID.     [SAMI]
>       - Add flags to define access privilege and attributes for      [SAMI]
>         the memory ranges.
>     v3:
>       - Submit patch series to update platform code to use the       [LIMING]
>         EFI_ACPI_IO_REMAPPING_TABLE_REV0 macro.
>         Ref: https://edk2.groups.io/g/devel/topic/83618423#76799
>       - Removed definition of EFI_ACPI_IO_REMAPPING_TABLE_REVISION   [SAMI]
>         as EFI_ACPI_IO_REMAPPING_TABLE_REV0 has been provided for
>         representing Rev 0. Also, a corresponding patch series
>         for updating the platforms in edk2-platforms repository
>         shall be submitted to the edk2 mailing list.
>       - Include r-b received from v2 series.                         [SAMI]
>         Ref: https://edk2.groups.io/g/devel/topic/83600724#76660
>
>     v2:
>       - Set EFI_ACPI_IO_REMAPPING_TABLE_REVISION to Rev 0 as     [SAMI]
>         setting to Rev 3 will break existing platforms. The
>         problem is that existing code would not be populating
>         the Identifier field in the nodes. This can lead to
>         non-unique values in the Identifier field.
>
>  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 83 ++++++++++++++++++--
>  1 file changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> index 79a34678681d45b2982dc8573db6bd447f42e429..07cb7f49dc936fb00cc549113f1e62f988535e5d 100644
> --- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> @@ -1,12 +1,19 @@
>  /** @file
> -  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049D
> -
> -  http://infocenter.arm.com/help/topic/com.arm.doc.den0049d/DEN0049D_IO_Remapping_Table.pdf
> +  ACPI IO Remapping Table (IORT) definitions.
>
>    Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
> -  Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
> +    (https://developer.arm.com/documentation/den0049/)
> +
> +  @par Glossary:
> +  - Ref  : Reference
> +  - Mem  : Memory
> +  - Desc : Descriptor
>  **/
>
>  #ifndef __IO_REMAPPING_TABLE_H__
> @@ -14,7 +21,8 @@
>
>  #include <IndustryStandard/Acpi.h>
>
> -#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV0  0x0
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV5  0x5
>
>  #define EFI_ACPI_IORT_TYPE_ITS_GROUP     0x0
>  #define EFI_ACPI_IORT_TYPE_NAMED_COMP    0x1
> @@ -22,6 +30,7 @@
>  #define EFI_ACPI_IORT_TYPE_SMMUv1v2      0x3
>  #define EFI_ACPI_IORT_TYPE_SMMUv3        0x4
>  #define EFI_ACPI_IORT_TYPE_PMCG          0x5
> +#define EFI_ACPI_IORT_TYPE_RMR           0x6
>
>  #define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA  BIT0
>
> @@ -55,7 +64,29 @@
>  #define EFI_ACPI_IORT_SMMUv3_MODEL_CAVIUM_CN99XX     0x2
>
>  #define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
> -#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    BIT0
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_SUPPORTED    BIT1
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_SUPPORTED    BIT2
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_SUPPORTED    BIT1
> +
> +#define EFI_ACPI_IORT_RMR_REMAP_NOT_PERMITTED  0x0
> +#define EFI_ACPI_IORT_RMR_REMAP_PERMITTED      BIT0
> +
> +#define EFI_ACPI_IORT_RMR_ACCESS_REQ_NOT_PRIVILEGED  0x0
> +#define EFI_ACPI_IORT_RMR_ACCESS_REQ_PRIVILEGED      BIT1
> +
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRNE             0x0
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRE              0x1
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGRE               0x2
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_GRE                0x3
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_NC_OUT_NC      0x4
> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_WB_OUT_WB_ISH  0x5
>
>  #define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE  BIT0
>
> @@ -89,7 +120,7 @@ typedef struct {
>    UINT8     Type;
>    UINT16    Length;
>    UINT8     Revision;
> -  UINT32    Reserved;
> +  UINT32    Identifier;
>    UINT32    NumIdMappings;
>    UINT32    IdReference;
>  } EFI_ACPI_6_0_IO_REMAPPING_NODE;
> @@ -118,7 +149,9 @@ typedef struct {
>    UINT32                            AtsAttribute;
>    UINT32                            PciSegmentNumber;
>    UINT8                             MemoryAddressSize;
> -  UINT8                             Reserved1[3];
> +  UINT16                            PasidCapabilities;
> +  UINT8                             Reserved1[1];
> +  UINT32                            Flags;
>  } EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
>
>  ///
> @@ -198,6 +231,40 @@ typedef struct {
>    // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE      OverflowInterruptMsiMapping[1];
>  } EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE;
>
> +///
> +/// Memory Range Descriptor.
> +///
> +typedef struct {
> +  /// Base address of Reserved Memory Range,
> +  /// aligned to a page size of 64K.
> +  UINT64    Base;
> +
> +  /// Length of the Reserved Memory range.
> +  /// Must be a multiple of the page size of 64K.
> +  UINT64    Length;
> +
> +  /// Reserved, must be zero.
> +  UINT32    Reserved;
> +} EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC;
> +
> +///
> +/// Node type 6: Reserved Memory Range (RMR) node
> +///
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE    Node;
> +
> +  /// RMR flags
> +  UINT32                            Flags;
> +
> +  /// Memory range descriptor count.
> +  UINT32                            NumMemRangeDesc;
> +
> +  /// Offset of the memory range descriptor array.
> +  UINT32                            MemRangeDescRef;
> +  // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE         IdMapping[1];
> +  // EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC   MemRangeDesc[1];
> +} EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE;
> +
>  #pragma pack()
>
>  #endif
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>

Please break out the Flags attribute to its own structure so it better
reflects the documentation in the spec.

I am current using something like this,

+^M
+///^M
+/// Node header definition shared by all node types^M
+///^M
+typedef struct {^M
+  UINT32                                  RemappingPermitted   : 1;
+  UINT32                                  AccessPrivilege      : 1;
+  UINT32                                  AccessAttributes     : 8;
+  UINT32                                  Reserved10_31        : 22;
+} EFI_ACPI_6_0_IO_REMAPPING_RMR_FLAGS;
+^M
 ///
 /// Node type 6: Reserved Memory Range (RMR) node
 ///
@@ -240,7 +258,7 @@ typedef struct {
   EFI_ACPI_6_0_IO_REMAPPING_NODE              Node;

   /// RMR flags
-  UINT32                                      Flags;
+  EFI_ACPI_6_0_IO_REMAPPING_RMR_FLAGS         Flags;

-Jon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 回复: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-07  1:51     ` 回复: " gaoliming
@ 2022-07-07 10:31       ` Sami Mujawar
  0 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-07 10:31 UTC (permalink / raw)
  To: gaoliming, devel
  Cc: Ard Biesheuvel (TianoCore), michael.d.kinney, zhiguang.liu,
	Steven Price, nd

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

Hi Liming,

Thank you for your feedback. Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 07/07/2022 02:51 am, gaoliming wrote:
>
> Sami:
>
>  I suggest to keep EFI_ACPI_IO_REMAPPING_TABLE_REVISION. Its value can 
> be EFI_ACPI_IO_REMAPPING_TABLE_REV5, because the structure has been 
> updated, such as EFI_ACPI_6_0_IO_REMAPPING_RC_NODE.
>
[SAMI] Ack. I will change this in the next patch series.
>
>  Compared to the previous version, EFI_ACPI_6_0_IO_REMAPPING_RC_NODE 
> struct is larger. Does this change bring any impact?
>
[SAMI] The EFI_ACPI_6_0_IO_REMAPPING_NODE.Length field should reflect 
the increased size. Similarly, the Revision field in the IORT ACPI table 
header and the EFI_ACPI_6_0_IO_REMAPPING_NODE.Revision field would also 
be updated accordingly. Therefore, an OS should be able to detect the 
change and handle accordinlgy.

As for existing firmware code that does not make use of the latest IORT 
revision, I have an edk2-platforms patch series that fixes the IORT 
Table revision to EFI_ACPI_IO_REMAPPING_TABLE_REV0, see 
https://edk2.groups.io/g/devel/message/91104.

[/SAMI]

>
> Thanks
>
> Liming
>
> *发件人:*devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Sami Mujawar
> *发送时间:*2022年7月6日23:44
> *收件人:*Sami Mujawar <sami.mujawar@arm.com>; devel@edk2.groups.io
> *主题:*Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for 
> IORT Rev E.d spec
>
> On Wed, Jul 6, 2022 at 02:57 AM, Sami Mujawar wrote:
>
>     -#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION 0x0
>     +#define EFI_ACPI_IO_REMAPPING_TABLE_REV0 0x0
>     +#define EFI_ACPI_IO_REMAPPING_TABLE_REV5 0x5
>
> Based on discussion at 
> https://edk2.groups.io/g/devel/topic/patch_edk2_platforms_v2/92203690?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,92203690,previd%3D1657121904386162709,nextid%3D1657101454237486469&previd=1657121904386162709&nextid=1657101454237486469, 
> <https://edk2.groups.io/g/devel/topic/patch_edk2_platforms_v2/92203690?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,92203690,previd%3D1657121904386162709,nextid%3D1657101454237486469&previd=1657121904386162709&nextid=1657101454237486469,> 
> the suggestion is to rename these macros to 
> EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00 and 
> EFI_ACPI_IO_REMAPPING_TABLE_REVISION_05 respectively.
> I will wait for any other feedback on this series, before I send out 
> the updated series reflecting the macro renaming.
>
> Regards,
>
> Sami Mujawar
>
> 
>

[-- Attachment #2: Type: text/html, Size: 8066 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec
  2022-07-07  6:05   ` Jon Nettleton
@ 2022-07-07 11:02     ` Sami Mujawar
  0 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2022-07-07 11:02 UTC (permalink / raw)
  To: Jon Nettleton, edk2-devel-groups-io, Ard Biesheuvel (TianoCore),
	Liming Gao, michael.d.kinney, zhiguang.liu
  Cc: Alexei.Fedorov, quic_llindhol, pierre.gondois, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, Steven Price, Lorenzo Pieralisi,
	ray.ni, zhichao.gao, nd

Hi Jon,

Thank you for your feedback. Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 07/07/2022 07:05 am, Jon Nettleton wrote:
> On Wed, Jul 6, 2022 at 11:57 AM Sami Mujawar <sami.mujawar@arm.com> wrote:
>> Bugzilla: 3458 - Add support IORT Rev E.d specification updates
>>            (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>>
>> The IO Remapping Table, Platform Design Document, Revision E.d,
>> Feb 2022 (https://developer.arm.com/documentation/den0049/)
>> introduces the following updates, collectively including the
>> updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
>>    - increments the IORT table revision to 5.
>>    - updates the node definition to add an 'Identifier' field.
>>    - adds definition of node type 6 - Reserved Memory Range node.
>>    - adds definition for Memory Range Descriptors.
>>    - adds flag to indicate PRI support for root complexes.
>>    - adds flag to indicate if the root complex supports forwarding
>>      of PASID information on translated transactions to the SMMU.
>>    - adds flag to indicate if the root complex supports PASID.
>>    - adds flags to define access privilege and attributes for the
>>      memory ranges.
>>
>> Therefore, update the IORT header file to reflect these changes,
>> and also rename the EFI_ACPI_IO_REMAPPING_TABLE_REVISION macro to
>> EFI_ACPI_IO_REMAPPING_TABLE_REV0.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>
>> Notes:
>>      v4:
>>        - Updated patch series to IORT specification revision E.d.     [SAMI]
>>        - Add flag to indicate if the root complex supports PASID.     [SAMI]
>>        - Add flags to define access privilege and attributes for      [SAMI]
>>          the memory ranges.
>>      v3:
>>        - Submit patch series to update platform code to use the       [LIMING]
>>          EFI_ACPI_IO_REMAPPING_TABLE_REV0 macro.
>>          Ref: https://edk2.groups.io/g/devel/topic/83618423#76799
>>        - Removed definition of EFI_ACPI_IO_REMAPPING_TABLE_REVISION   [SAMI]
>>          as EFI_ACPI_IO_REMAPPING_TABLE_REV0 has been provided for
>>          representing Rev 0. Also, a corresponding patch series
>>          for updating the platforms in edk2-platforms repository
>>          shall be submitted to the edk2 mailing list.
>>        - Include r-b received from v2 series.                         [SAMI]
>>          Ref: https://edk2.groups.io/g/devel/topic/83600724#76660
>>
>>      v2:
>>        - Set EFI_ACPI_IO_REMAPPING_TABLE_REVISION to Rev 0 as     [SAMI]
>>          setting to Rev 3 will break existing platforms. The
>>          problem is that existing code would not be populating
>>          the Identifier field in the nodes. This can lead to
>>          non-unique values in the Identifier field.
>>
>>   MdePkg/Include/IndustryStandard/IoRemappingTable.h | 83 ++++++++++++++++++--
>>   1 file changed, 75 insertions(+), 8 deletions(-)
>>
>> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> index 79a34678681d45b2982dc8573db6bd447f42e429..07cb7f49dc936fb00cc549113f1e62f988535e5d 100644
>> --- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> @@ -1,12 +1,19 @@
>>   /** @file
>> -  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049D
>> -
>> -  http://infocenter.arm.com/help/topic/com.arm.doc.den0049d/DEN0049D_IO_Remapping_Table.pdf
>> +  ACPI IO Remapping Table (IORT) definitions.
>>
>>     Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
>> -  Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
>> +  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.<BR>
>>
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +  @par Reference(s):
>> +  - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
>> +    (https://developer.arm.com/documentation/den0049/)
>> +
>> +  @par Glossary:
>> +  - Ref  : Reference
>> +  - Mem  : Memory
>> +  - Desc : Descriptor
>>   **/
>>
>>   #ifndef __IO_REMAPPING_TABLE_H__
>> @@ -14,7 +21,8 @@
>>
>>   #include <IndustryStandard/Acpi.h>
>>
>> -#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  0x0
>> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV0  0x0
>> +#define EFI_ACPI_IO_REMAPPING_TABLE_REV5  0x5
>>
>>   #define EFI_ACPI_IORT_TYPE_ITS_GROUP     0x0
>>   #define EFI_ACPI_IORT_TYPE_NAMED_COMP    0x1
>> @@ -22,6 +30,7 @@
>>   #define EFI_ACPI_IORT_TYPE_SMMUv1v2      0x3
>>   #define EFI_ACPI_IORT_TYPE_SMMUv3        0x4
>>   #define EFI_ACPI_IORT_TYPE_PMCG          0x5
>> +#define EFI_ACPI_IORT_TYPE_RMR           0x6
>>
>>   #define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA  BIT0
>>
>> @@ -55,7 +64,29 @@
>>   #define EFI_ACPI_IORT_SMMUv3_MODEL_CAVIUM_CN99XX     0x2
>>
>>   #define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
>> -#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    BIT0
>> +
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_UNSUPPORTED  0x0
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PRI_SUPPORTED    BIT1
>> +
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_UNSUPPORTED  0x0
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_FWD_SUPPORTED    BIT2
>> +
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_UNSUPPORTED  0x0
>> +#define EFI_ACPI_IORT_ROOT_COMPLEX_PASID_SUPPORTED    BIT1
>> +
>> +#define EFI_ACPI_IORT_RMR_REMAP_NOT_PERMITTED  0x0
>> +#define EFI_ACPI_IORT_RMR_REMAP_PERMITTED      BIT0
>> +
>> +#define EFI_ACPI_IORT_RMR_ACCESS_REQ_NOT_PRIVILEGED  0x0
>> +#define EFI_ACPI_IORT_RMR_ACCESS_REQ_PRIVILEGED      BIT1
>> +
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRNE             0x0
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGNRE              0x1
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_NGRE               0x2
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_DEV_GRE                0x3
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_NC_OUT_NC      0x4
>> +#define EFI_ACPI_IORT_RMR_ACCESS_ATTRIB_NORM_IN_WB_OUT_WB_ISH  0x5
>>
>>   #define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE  BIT0
>>
>> @@ -89,7 +120,7 @@ typedef struct {
>>     UINT8     Type;
>>     UINT16    Length;
>>     UINT8     Revision;
>> -  UINT32    Reserved;
>> +  UINT32    Identifier;
>>     UINT32    NumIdMappings;
>>     UINT32    IdReference;
>>   } EFI_ACPI_6_0_IO_REMAPPING_NODE;
>> @@ -118,7 +149,9 @@ typedef struct {
>>     UINT32                            AtsAttribute;
>>     UINT32                            PciSegmentNumber;
>>     UINT8                             MemoryAddressSize;
>> -  UINT8                             Reserved1[3];
>> +  UINT16                            PasidCapabilities;
>> +  UINT8                             Reserved1[1];
>> +  UINT32                            Flags;
>>   } EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
>>
>>   ///
>> @@ -198,6 +231,40 @@ typedef struct {
>>     // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE      OverflowInterruptMsiMapping[1];
>>   } EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE;
>>
>> +///
>> +/// Memory Range Descriptor.
>> +///
>> +typedef struct {
>> +  /// Base address of Reserved Memory Range,
>> +  /// aligned to a page size of 64K.
>> +  UINT64    Base;
>> +
>> +  /// Length of the Reserved Memory range.
>> +  /// Must be a multiple of the page size of 64K.
>> +  UINT64    Length;
>> +
>> +  /// Reserved, must be zero.
>> +  UINT32    Reserved;
>> +} EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC;
>> +
>> +///
>> +/// Node type 6: Reserved Memory Range (RMR) node
>> +///
>> +typedef struct {
>> +  EFI_ACPI_6_0_IO_REMAPPING_NODE    Node;
>> +
>> +  /// RMR flags
>> +  UINT32                            Flags;
>> +
>> +  /// Memory range descriptor count.
>> +  UINT32                            NumMemRangeDesc;
>> +
>> +  /// Offset of the memory range descriptor array.
>> +  UINT32                            MemRangeDescRef;
>> +  // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE         IdMapping[1];
>> +  // EFI_ACPI_6_0_IO_REMAPPING_MEM_RANGE_DESC   MemRangeDesc[1];
>> +} EFI_ACPI_6_0_IO_REMAPPING_RMR_NODE;
>> +
>>   #pragma pack()
>>
>>   #endif
>> --
>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>
>>
>>
> Please break out the Flags attribute to its own structure so it better
> reflects the documentation in the spec.
>
> I am current using something like this,
>
> +^M
> +///^M
> +/// Node header definition shared by all node types^M
> +///^M
> +typedef struct {^M
> +  UINT32                                  RemappingPermitted   : 1;
> +  UINT32                                  AccessPrivilege      : 1;
> +  UINT32                                  AccessAttributes     : 8;
> +  UINT32                                  Reserved10_31        : 22;
> +} EFI_ACPI_6_0_IO_REMAPPING_RMR_FLAGS;
> +^M
>   ///
>   /// Node type 6: Reserved Memory Range (RMR) node
>   ///
> @@ -240,7 +258,7 @@ typedef struct {
>     EFI_ACPI_6_0_IO_REMAPPING_NODE              Node;
>
>     /// RMR flags
> -  UINT32                                      Flags;
> +  EFI_ACPI_6_0_IO_REMAPPING_RMR_FLAGS         Flags;

[SAMI] I have followed the coding style as in rest of this file. My 
concern is that introducing the use of bit fields for RMR node would 
make it inconsistent with the rest of the file.

Also, updating the Flags field in other IORT nodes to use bit fileds, 
would result in having to make corresponding changes to the platform code.

Considering this, I don't think this is something we should do.

[/SAMI]
> -Jon

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-07-07 11:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-06  9:56 [PATCH v4 0/8] IORT Rev E.d specification updates Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 1/8] MdePkg: IORT header update for IORT Rev E.d spec Sami Mujawar
2022-07-06 15:44   ` [edk2-devel] " Sami Mujawar
2022-07-07  1:51     ` 回复: " gaoliming
2022-07-07 10:31       ` Sami Mujawar
2022-07-07  6:05   ` Jon Nettleton
2022-07-07 11:02     ` Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 2/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment Sami Mujawar
2022-07-06  9:56 ` [PATCH 3/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 4/8] DynamicTablesPkg: Handle error when IdMappingToken is NULL Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 5/8] DynamicTablesPkg: IORT set reference to Id array only if present Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 6/8] DynamicTablesPkg: IORT set reference to interrupt array " Sami Mujawar
2022-07-06  9:56 ` [PATCH v4 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.d Sami Mujawar
2022-07-06  9:56 ` [PATCH 8/8] DynamicTablesPkg: IORT generator updates for Rev E.d spec Sami Mujawar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox