[PATCH v2 0/8] IORT Rev E.b specification updates

[PATCH v2 0/8] IORT Rev E.b specification updates

[Sami Mujawar]

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

The IO Remapping Table (IORT) specification has been updated to
rev E.b. The following updates are introduced including the errata
to rev E and E.a:
  - increments the IORT table revision to 3.
  - 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.

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.

This 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 changes can be seen at:
https://github.com/samimujawar/edk2/tree/1527_iort_rev_eb_v2

Sami Mujawar (8):
  MdePkg: Fix IORT header file include guard
  MdePkg: IORT header update for IORT Rev E.b spec
  ShellPkg: Acpiview: Abbreviate field names to preserve alignment
  ShellPkg: Acpiview: IORT parser update for IORT Rev E.b spec
  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.b
  DynamicTablesPkg: IORT generator updates for Rev E.b spec

 DynamicTablesPkg/Include/ArmNameSpaceObjects.h                         |  58 ++
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c       | 772 ++++++++++++++++++--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h       |   5 +-
 MdePkg/Include/IndustryStandard/IoRemappingTable.h                     |  71 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 207 +++++-
 5 files changed, 1013 insertions(+), 100 deletions(-)

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

[PATCH v2 1/8] MdePkg: Fix IORT header file include guard

[Sami Mujawar]

According to section 5.3.5, EDK II C Coding Standards Specification
(https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification)
the header file guard names must not be prefixed with underscores as
they are reserved for compiler implementation.

Therefore, fix the header file include guard as per the specification
guidelines.

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

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

 MdePkg/Include/IndustryStandard/IoRemappingTable.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
index 90504e3a6715be7facc6450c6ff0e1eab92cd3c7..731217441438a00dd5ff0bedf2010598d48d6dbf 100644
--- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
+++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
@@ -9,8 +9,8 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-#ifndef __IO_REMAPPING_TABLE_H__
-#define __IO_REMAPPING_TABLE_H__
+#ifndef IO_REMAPPING_TABLE_H_
+#define IO_REMAPPING_TABLE_H_
 
 #include <IndustryStandard/Acpi.h>
 
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v2 1/8] MdePkg: Fix IORT header file include guard

[Michael D Kinney]

Hi Sami,

The include guard pattern is present everywhere.  No sure it makes sense to start fixing these one at a time.

The #pragma once may be a better long term solution and may improve build times slightly.

	https://en.wikipedia.org/wiki/Pragma_once

Best regards,

Mike

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, June 17, 2021 2:56 AM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com; ardb+tianocore@kernel.org; Matteo.Carlini@arm.com;
> Ben.Adderson@arm.com; Kinney, Michael D <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; nd@arm.com
> Subject: [PATCH v2 1/8] MdePkg: Fix IORT header file include guard
> 
> According to section 5.3.5, EDK II C Coding Standards Specification
> (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification)
> the header file guard names must not be prefixed with underscores as
> they are reserved for compiler implementation.
> 
> Therefore, fix the header file include guard as per the specification
> guidelines.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
> 
>  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> index 90504e3a6715be7facc6450c6ff0e1eab92cd3c7..731217441438a00dd5ff0bedf2010598d48d6dbf 100644
> --- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> @@ -9,8 +9,8 @@
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> 
> -#ifndef __IO_REMAPPING_TABLE_H__
> -#define __IO_REMAPPING_TABLE_H__
> +#ifndef IO_REMAPPING_TABLE_H_
> +#define IO_REMAPPING_TABLE_H_
> 
>  #include <IndustryStandard/Acpi.h>
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v2 1/8] MdePkg: Fix IORT header file include guard

[Sami Mujawar]

Hi Mike,

I agree the use of the include guard is not consistent across edk2 code and it may be better to fix them all at once. However, if we decide to use '#pragma once', then the edk2 coding standard specification would need to be updated first. Similarly, the ECC tool would also need to be updated.

I can drop this change for now. Please let me know how you wish to proceed.

Regards,

Sami Mujawar

On 17/06/2021, 19:19, "devel@edk2.groups.io on behalf of Michael D Kinney via groups.io" <devel@edk2.groups.io on behalf of michael.d.kinney=intel.com@groups.io> wrote:

    Hi Sami,

    The include guard pattern is present everywhere.  No sure it makes sense to start fixing these one at a time.

    The #pragma once may be a better long term solution and may improve build times slightly.

    	https://en.wikipedia.org/wiki/Pragma_once

    Best regards,

    Mike

    > -----Original Message-----
    > From: Sami Mujawar <sami.mujawar@arm.com>
    > Sent: Thursday, June 17, 2021 2:56 AM
    > To: devel@edk2.groups.io
    > Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com; ardb+tianocore@kernel.org; Matteo.Carlini@arm.com;
    > Ben.Adderson@arm.com; Kinney, Michael D <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn; Liu, Zhiguang
    > <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; nd@arm.com
    > Subject: [PATCH v2 1/8] MdePkg: Fix IORT header file include guard
    > 
    > According to section 5.3.5, EDK II C Coding Standards Specification
    > (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification)
    > the header file guard names must not be prefixed with underscores as
    > they are reserved for compiler implementation.
    > 
    > Therefore, fix the header file include guard as per the specification
    > guidelines.
    > 
    > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
    > ---
    > 
    > Notes:
    >     v2:
    >      - No code change since v1. Re-sending with v2 series.    [SAMI]
    > 
    >  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 4 ++--
    >  1 file changed, 2 insertions(+), 2 deletions(-)
    > 
    > diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
    > index 90504e3a6715be7facc6450c6ff0e1eab92cd3c7..731217441438a00dd5ff0bedf2010598d48d6dbf 100644
    > --- a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
    > +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
    > @@ -9,8 +9,8 @@
    >    SPDX-License-Identifier: BSD-2-Clause-Patent
    >  **/
    > 
    > -#ifndef __IO_REMAPPING_TABLE_H__
    > -#define __IO_REMAPPING_TABLE_H__
    > +#ifndef IO_REMAPPING_TABLE_H_
    > +#define IO_REMAPPING_TABLE_H_
    > 
    >  #include <IndustryStandard/Acpi.h>
    > 
    > --
    > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



    

[PATCH v2 2/8] MdePkg: IORT header update for IORT Rev E.b spec

[Sami Mujawar]

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

The IO Remapping Table, Platform Design Document, Revision E.b,
Feb 2021 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E and Rev E.a:
  - increments the IORT table revision to 3.
  - 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.

Therefore, update the IORT header file to reflect these changes.

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

Notes:
    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 | 67 ++++++++++++++++++--
 1 file changed, 60 insertions(+), 7 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
index 731217441438a00dd5ff0bedf2010598d48d6dbf..a9817252d8cec17f82cb1a4ced12186cdf58713a 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 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021
+    (https://developer.arm.com/documentation/den0049/)
+
+  @par Glossary:
+  - Ref  : Reference
+  - Mem  : Memory
+  - Desc : Descriptor
 **/
 
 #ifndef IO_REMAPPING_TABLE_H_
@@ -14,7 +21,9 @@
 
 #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_REV3      0x3
+#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  EFI_ACPI_IO_REMAPPING_TABLE_REV0
 
 #define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
 #define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
@@ -22,6 +31,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 +65,16 @@
 #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_RMR_REMAP_NOT_PERMITTED       0x0
+#define EFI_ACPI_IORT_RMR_REMAP_PERMITTED           BIT0
 
 #define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
 
@@ -89,7 +108,7 @@ typedef struct {
   UINT8                                   Type;
   UINT16                                  Length;
   UINT8                                   Revision;
-  UINT32                                  Reserved;
+  UINT32                                  Identifier;
   UINT32                                  NumIdMappings;
   UINT32                                  IdReference;
 } EFI_ACPI_6_0_IO_REMAPPING_NODE;
@@ -198,6 +217,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)'

Re: [PATCH v2 2/8] MdePkg: IORT header update for IORT Rev E.b spec

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, June 17, 2021 5:56 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com;
> ardb+tianocore@kernel.org; Matteo.Carlini@arm.com;
> Ben.Adderson@arm.com; steven.price@arm.com;
> Lorenzo.Pieralisi@arm.com; Kinney, Michael D
> <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; nd@arm.com
> Subject: [PATCH v2 2/8] MdePkg: IORT header update for IORT Rev E.b spec
> 
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
> 
> The IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021
> (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the updates and
> errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
> 
> Therefore, update the IORT header file to reflect these changes.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>     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 | 67
> ++++++++++++++++++--
>  1 file changed, 60 insertions(+), 7 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> index
> 731217441438a00dd5ff0bedf2010598d48d6dbf..a9817252d8cec17f82cb1a4ced
> 12186cdf58713a 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_I
> O_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 - 2021, Arm Limited. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021
> +    (https://developer.arm.com/documentation/den0049/)
> +
> +  @par Glossary:
> +  - Ref  : Reference
> +  - Mem  : Memory
> +  - Desc : Descriptor
>  **/
> 
>  #ifndef IO_REMAPPING_TABLE_H_
> @@ -14,7 +21,9 @@
> 
>  #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_REV3      0x3
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION
> +EFI_ACPI_IO_REMAPPING_TABLE_REV0
> 
>  #define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
>  #define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
> @@ -22,6 +31,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 +65,16 @@
>  #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_RMR_REMAP_NOT_PERMITTED       0x0
> +#define EFI_ACPI_IORT_RMR_REMAP_PERMITTED           BIT0
> 
>  #define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
> 
> @@ -89,7 +108,7 @@ typedef struct {
>    UINT8                                   Type;
>    UINT16                                  Length;
>    UINT8                                   Revision;
> -  UINT32                                  Reserved;
> +  UINT32                                  Identifier;
>    UINT32                                  NumIdMappings;
>    UINT32                                  IdReference;
>  } EFI_ACPI_6_0_IO_REMAPPING_NODE;
> @@ -198,6 +217,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)'

Re: [edk2-devel] [PATCH v2 2/8] MdePkg: IORT header update for IORT Rev E.b spec

[PierreGondois]

Hi Sami,

Just a minor comment:

On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.b,
> Feb 2021 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
>
> Therefore, update the IORT header file to reflect these changes.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
>     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 | 67 ++++++++++++++++++--
>  1 file changed, 60 insertions(+), 7 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> index 731217441438a00dd5ff0bedf2010598d48d6dbf..a9817252d8cec17f82cb1a4ced12186cdf58713a 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 - 2021, Arm Limited. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021
> +    (https://developer.arm.com/documentation/den0049/)
> +
> +  @par Glossary:
> +  - Ref  : Reference
> +  - Mem  : Memory
> +  - Desc : Descriptor
>  **/
>  
>  #ifndef IO_REMAPPING_TABLE_H_
> @@ -14,7 +21,9 @@
>  
>  #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_REV3      0x3
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION  EFI_ACPI_IO_REMAPPING_TABLE_REV0

[Pierre]

Maybe this would be good to add a comment to explain why
EFI_ACPI_IO_REMAPPING_TABLE_REVISION points to
EFI_ACPI_IO_REMAPPING_TABLE_REV0.

Cf the notes of this patch:
    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.

>  
>  #define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
>  #define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
> @@ -22,6 +31,7 @@
>  #define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
>  #define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
>  #define EFI_ACPI_IORT_TYPE_PMCG                     0x5
>
[snip]

[PATCH v2 3/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment

[Sami Mujawar]

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>
---

Notes:
    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 f7447947b2308d35d4d2890373778f0fd2f97f9e..fcecaff5134256497bda87241f339076897c3ece 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)'

Re: [PATCH v2 3/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, June 17, 2021 5:56 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com;
> Matteo.Carlini@arm.com; Ben.Adderson@arm.com; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; nd@arm.com
> Subject: [PATCH v2 3/8] ShellPkg: Acpiview: Abbreviate field names to
> preserve alignment
> 
> 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>
> ---
> 
> Notes:
>     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
> f7447947b2308d35d4d2890373778f0fd2f97f9e..fcecaff5134256497bda87241f3
> 39076897c3ece 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortPars
> +++ er.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)'

Re: [edk2-devel] [PATCH v2 3/8] ShellPkg: Acpiview: Abbreviate field names to preserve alignment

[PierreGondois]

Hi Sami,

The patch looks good to me:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Regards,
Pierre


On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> 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>
> ---
>
> Notes:
>     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 f7447947b2308d35d4d2890373778f0fd2f97f9e..fcecaff5134256497bda87241f339076897c3ece 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

[PATCH v2 4/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.b spec

[Sami Mujawar]

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

The IO Remapping Table, Platform Design Document, Revision E.b,
Feb 2021 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E and Rev E.a:
  - increments the IORT table revision to 3.
  - 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.

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 ID mapping count is
    set to 1.

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

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

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

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index fcecaff5134256497bda87241f339076897c3ece..1507dd3a4d79e61024b0c5526e21ffdacb782251 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -5,10 +5,12 @@
   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.b, Feb 2021
+      (https://developer.arm.com/documentation/den0049/)
 
   @par Glossary:
     - Ref  - Reference
+    - Desc - Descriptor
 **/
 
 #include <IndustryStandard/IoRemappingTable.h>
@@ -36,6 +38,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 +105,72 @@ ValidateItsIdArrayReference (
   }
 }
 
+/**
+  This function validates that the address or length 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
+Validate64KAlignment (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT64 Address;
+  Address = *(UINT64*)Ptr;
+  if ((Address & (SIZE_64KB - 1)) != 0) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: Value must be 64K aligned.");
+  }
+}
+
+/**
+  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.");
+  }
+}
+
+/**
+  This function validates the ID Mapping array count for the Reserved
+  Memory Range (RMR) node.
+
+  @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
+ValidateRmrIdMappingCount (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  if (*(UINT32*)Ptr != 1) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: IORT ID Mapping count must be set to 1.");
+  }
+}
+
 /**
   Helper Macro for populating the IORT Node header in the ACPI_PARSER array.
 
@@ -113,7 +184,7 @@ ValidateItsIdArrayReference (
   { 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"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,                      \
@@ -253,6 +324,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 (ValidateRmrIdMappingCount, 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, Validate64KAlignment,
+   NULL},
+  {L"Physical Range length", 8, 8, L"0x%lx", NULL, NULL, Validate64KAlignment,
+   NULL},
+  {L"Reserved", 4, 16, L"0x%x", NULL, NULL, NULL, NULL}
+};
+
 /**
   This function parses the IORT Node Id Mapping array.
 
@@ -601,9 +695,93 @@ 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)
+    );
+
+  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
@@ -612,6 +790,7 @@ DumpIortNodePmcg (
     - SMMUv1/2
     - SMMUv3
     - PMCG
+    - RMR
 
   This function also performs validation of the ACPI table fields.
 
@@ -753,9 +932,16 @@ ParseAcpiIort (
           *IortNodeLength,
           *IortIdMappingCount,
           *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)'

Re: [PATCH v2 4/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.b spec

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, June 17, 2021 5:56 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com;
> steven.price@arm.com; Lorenzo.Pieralisi@arm.com;
> Matteo.Carlini@arm.com; Ben.Adderson@arm.com; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; nd@arm.com
> Subject: [PATCH v2 4/8] ShellPkg: Acpiview: IORT parser update for IORT Rev
> E.b spec
> 
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
> 
> The IO Remapping Table, Platform Design Document, Revision E.b, Feb 2021
> (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the updates and
> errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
> 
> 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 ID mapping count is
>     set to 1.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |
> 196 +++++++++++++++++++-
>  1 file changed, 191 insertions(+), 5 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> index
> fcecaff5134256497bda87241f339076897c3ece..1507dd3a4d79e61024b0c5526e
> 21ffdacb782251 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortPars
> +++ er.c
> @@ -5,10 +5,12 @@
>    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.b, Feb 2021
> +      (https://developer.arm.com/documentation/den0049/)
> 
>    @par Glossary:
>      - Ref  - Reference
> +    - Desc - Descriptor
>  **/
> 
>  #include <IndustryStandard/IoRemappingTable.h>
> @@ -36,6 +38,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 +105,72 @@ ValidateItsIdArrayReference (
>    }
>  }
> 
> +/**
> +  This function validates that the address or length 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
> +Validate64KAlignment (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  UINT64 Address;
> +  Address = *(UINT64*)Ptr;
> +  if ((Address & (SIZE_64KB - 1)) != 0) {
> +    IncrementErrorCount ();
> +    Print (L"\nERROR: Value must be 64K aligned.");
> +  }
> +}
> +
> +/**
> +  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.");
> +  }
> +}
> +
> +/**
> +  This function validates the ID Mapping array count for the Reserved
> +  Memory Range (RMR) node.
> +
> +  @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
> +ValidateRmrIdMappingCount (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  if (*(UINT32*)Ptr != 1) {
> +    IncrementErrorCount ();
> +    Print (L"\nERROR: IORT ID Mapping count must be set to 1.");
> +  }
> +}
> +
>  /**
>    Helper Macro for populating the IORT Node header in the ACPI_PARSER
> array.
> 
> @@ -113,7 +184,7 @@ ValidateItsIdArrayReference (
>    { 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"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,                      \
> @@ -253,6 +324,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 (ValidateRmrIdMappingCount, 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,
> Validate64KAlignment,
> +   NULL},
> +  {L"Physical Range length", 8, 8, L"0x%lx", NULL, NULL,
> Validate64KAlignment,
> +   NULL},
> +  {L"Reserved", 4, 16, L"0x%x", NULL, NULL, NULL, NULL} };
> +
>  /**
>    This function parses the IORT Node Id Mapping array.
> 
> @@ -601,9 +695,93 @@ 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)
> +    );
> +
> +  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
> @@ -612,6 +790,7 @@ DumpIortNodePmcg (
>      - SMMUv1/2
>      - SMMUv3
>      - PMCG
> +    - RMR
> 
>    This function also performs validation of the ACPI table fields.
> 
> @@ -753,9 +932,16 @@ ParseAcpiIort (
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *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)'

[PATCH v2 5/8] DynamicTablesPkg: IORT set reference to Id array only if present

[Sami Mujawar]

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>
---

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

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

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index 349caa8006bc34ca789cb3e321a0f87c0cd4ff0d..bdf839eab25e2b84b40c50da38f2bf961cdc5f42 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 - 2021, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -870,9 +870,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;
@@ -990,7 +990,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;
@@ -1160,11 +1161,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;
@@ -1319,8 +1321,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;
@@ -1438,7 +1440,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)'

Re: [edk2-devel] [PATCH v2 5/8] DynamicTablesPkg: IORT set reference to Id array only if present

[PierreGondois]

Hi Sami,

The patch looks good to me:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Regards,
Pierre

On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> 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>
> ---
>
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
>
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 29 +++++++++++---------
>  1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> index 349caa8006bc34ca789cb3e321a0f87c0cd4ff0d..bdf839eab25e2b84b40c50da38f2bf961cdc5f42 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 - 2021, Arm Limited. All rights reserved.
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>    @par Reference(s):
> @@ -870,9 +870,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;
> @@ -990,7 +990,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;
> @@ -1160,11 +1161,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;
> @@ -1319,8 +1321,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;
> @@ -1438,7 +1440,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;

[PATCH v2 6/8] DynamicTablesPkg: IORT set reference to interrupt array if present

[Sami Mujawar]

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>
---

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

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

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index bdf839eab25e2b84b40c50da38f2bf961cdc5f42..9ccf72594db378878d4e3abbafe98e749d9963da 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -1136,6 +1136,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);
 
@@ -1178,47 +1179,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));
+    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);
+    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)'

Re: [edk2-devel] [PATCH v2 6/8] DynamicTablesPkg: IORT set reference to interrupt array if present

[PierreGondois]

Hi Sami,

With the small modification:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>


On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> 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>
> ---
>
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
>
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 82 +++++++++++++-------
>  1 file changed, 55 insertions(+), 27 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> index bdf839eab25e2b84b40c50da38f2bf961cdc5f42..9ccf72594db378878d4e3abbafe98e749d9963da 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> @@ -1136,6 +1136,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);
>  
> @@ -1178,47 +1179,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));
> +    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);
> +    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;
> +      }
> +

Some similar modifications are done in the last patch of the serie:

[PATCH v2 8/8] DynamicTablesPkg: IORT generator updates for Rev E.b spec

For instance, the same change is done in the AddPmcgNodes() function.
Would it be possible to group them ?

>        Status = AddSmmuInterruptArray (
>                   CfgMgrProtocol,
>                   PmuInterruptArray,

[PATCH v2 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.b

[Sami Mujawar]

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

The IO Remapping Table, Platform Design Document, Revision E.b,
Feb 2021 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E and Rev E.a:
  - increments the IORT table revision to 3.
  - 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.

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.

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

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

 DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 58 ++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 19dcae13b2191e5f0b03ea85edec1191d2a406bf..98143cb5df127273cdd75452170fa651372b3896 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -58,6 +58,8 @@ typedef enum ArmObjectID {
   EArmObjGenericInitiatorAffinityInfo, ///< 34 - Generic Initiator Affinity
   EArmObjSerialPortInfo,               ///< 35 - Generic Serial Port Info
   EArmObjCmn600Info,                   ///< 36 - CMN-600 Info
+  EArmObjRmr,                          ///< 37 - Reserved Memory Range Node
+  EArmObjMemoryRangeDescriptor,        ///< 38 - Memory Range Descriptor
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -466,6 +468,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
@@ -497,6 +502,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
@@ -525,6 +533,9 @@ typedef struct CmArmRootComplexNode {
   UINT32            PciSegmentNumber;
   /// Memory address size limit
   UINT8             MemoryAddressSize;
+
+  /// Unique identifier for this node.
+  UINT32            Identifier;
 } CM_ARM_ROOT_COMPLEX_NODE;
 
 /** A structure that describes the
@@ -567,6 +578,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
@@ -603,6 +617,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
@@ -627,6 +644,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
@@ -878,6 +898,44 @@ typedef struct CmArmCmn600Info {
   CM_ARM_EXTENDED_INTERRUPT  DtcInterrupt[4];
 } CM_ARM_CMN_600_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;
+
+  /// Reserved Memory Range flags.
+  UINT32            Flags;
+
+  /// Memory range descriptor count.
+  UINT32            MemRangeDescCount;
+  /// Reference token for the Memory Range descriptor array
+  CM_OBJECT_TOKEN   MemRangeDescToken;
+
+  /// Unique identifier for this node.
+  UINT32            Identifier;
+} CM_ARM_RMR_NODE;
+
+/** A structure that describes the
+    Memory Range descriptor.
+
+    ID: EArmObjMemoryRangeDescriptor
+*/
+typedef struct CmArmRmrDescriptor {
+  /// Base address.
+  UINT64            BaseAddress;
+
+  /// Length.
+  UINT64            Length;
+} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
+
 #pragma pack()
 
 #endif // ARM_NAMESPACE_OBJECTS_H_
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v2 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.b

[PierreGondois]

Hi Sami,

With the small modifications:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>


On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.b,
> Feb 2021 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
>
> 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.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
>
>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 58 ++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 19dcae13b2191e5f0b03ea85edec1191d2a406bf..98143cb5df127273cdd75452170fa651372b3896 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -58,6 +58,8 @@ typedef enum ArmObjectID {
>    EArmObjGenericInitiatorAffinityInfo, ///< 34 - Generic Initiator Affinity
>    EArmObjSerialPortInfo,               ///< 35 - Generic Serial Port Info
>    EArmObjCmn600Info,                   ///< 36 - CMN-600 Info
> +  EArmObjRmr,                          ///< 37 - Reserved Memory Range Node
> +  EArmObjMemoryRangeDescriptor,        ///< 38 - Memory Range Descriptor
>    EArmObjMax
>  } EARM_OBJECT_ID;
>  
> @@ -466,6 +468,9 @@ typedef struct CmArmItsGroupNode {
>    UINT32            ItsIdCount;
>    /// Reference token for the ITS identifier array
>    CM_OBJECT_TOKEN   ItsIdToken;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;

Would it be possible to say in the description that this is part of the
common node header ?

As we cannot modify the order of the fields anymore, I understand this
cannot be upper, but this would help as the order is a bit confusing.

>  } CM_ARM_ITS_GROUP_NODE;
>  
>  /** A structure that describes the
> @@ -497,6 +502,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
> @@ -525,6 +533,9 @@ typedef struct CmArmRootComplexNode {
>    UINT32            PciSegmentNumber;
>    /// Memory address size limit
>    UINT8             MemoryAddressSize;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;
>  } CM_ARM_ROOT_COMPLEX_NODE;
>  
>  /** A structure that describes the
> @@ -567,6 +578,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
> @@ -603,6 +617,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
> @@ -627,6 +644,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
> @@ -878,6 +898,44 @@ typedef struct CmArmCmn600Info {
>    CM_ARM_EXTENDED_INTERRUPT  DtcInterrupt[4];
>  } CM_ARM_CMN_600_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;
> +
> +  /// Reserved Memory Range flags.
> +  UINT32            Flags;
> +
> +  /// Memory range descriptor count.
> +  UINT32            MemRangeDescCount;
> +  /// Reference token for the Memory Range descriptor array
> +  CM_OBJECT_TOKEN   MemRangeDescToken;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;
The 'Identifier' field should be added above the 'Flags' as the
structure is a new and to respect the order of the spec.
> +} CM_ARM_RMR_NODE;
> +
> +/** A structure that describes the
> +    Memory Range descriptor.
> +
> +    ID: EArmObjMemoryRangeDescriptor
> +*/
> +typedef struct CmArmRmrDescriptor {
> +  /// Base address.
> +  UINT64            BaseAddress;
> +
> +  /// Length.
> +  UINT64            Length;

The exact names of the fields in the spec are:
-Physical Range offset
-Physical Range length

Would it be possible to add these names in the description ?
> +} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
> +
>  #pragma pack()
>  
>  #endif // ARM_NAMESPACE_OBJECTS_H_

Re: [edk2-devel] [PATCH v2 7/8] DynamicTablesPkg: Update ArmNameSpaceObjects for IORT Rev E.b

[PierreGondois]

Hi Sami,

With the small modifications:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>


On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.b,
> Feb 2021 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
>
> 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.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
>     v2:
>      - No code change since v1. Re-sending with v2 series.    [SAMI]
>
>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 58 ++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 19dcae13b2191e5f0b03ea85edec1191d2a406bf..98143cb5df127273cdd75452170fa651372b3896 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -58,6 +58,8 @@ typedef enum ArmObjectID {
>    EArmObjGenericInitiatorAffinityInfo, ///< 34 - Generic Initiator Affinity
>    EArmObjSerialPortInfo,               ///< 35 - Generic Serial Port Info
>    EArmObjCmn600Info,                   ///< 36 - CMN-600 Info
> +  EArmObjRmr,                          ///< 37 - Reserved Memory Range Node
> +  EArmObjMemoryRangeDescriptor,        ///< 38 - Memory Range Descriptor
>    EArmObjMax
>  } EARM_OBJECT_ID;
>  
> @@ -466,6 +468,9 @@ typedef struct CmArmItsGroupNode {
>    UINT32            ItsIdCount;
>    /// Reference token for the ITS identifier array
>    CM_OBJECT_TOKEN   ItsIdToken;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;

Would it be possible to say in the description that this is part of the
common node header ?

As we cannot modify the order of the fields anymore, I understand this
cannot be upper, but this would help as the order is a bit confusing.

>  } CM_ARM_ITS_GROUP_NODE;
>  
>  /** A structure that describes the
> @@ -497,6 +502,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
> @@ -525,6 +533,9 @@ typedef struct CmArmRootComplexNode {
>    UINT32            PciSegmentNumber;
>    /// Memory address size limit
>    UINT8             MemoryAddressSize;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;
>  } CM_ARM_ROOT_COMPLEX_NODE;
>  
>  /** A structure that describes the
> @@ -567,6 +578,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
> @@ -603,6 +617,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
> @@ -627,6 +644,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
> @@ -878,6 +898,44 @@ typedef struct CmArmCmn600Info {
>    CM_ARM_EXTENDED_INTERRUPT  DtcInterrupt[4];
>  } CM_ARM_CMN_600_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;
> +
> +  /// Reserved Memory Range flags.
> +  UINT32            Flags;
> +
> +  /// Memory range descriptor count.
> +  UINT32            MemRangeDescCount;
> +  /// Reference token for the Memory Range descriptor array
> +  CM_OBJECT_TOKEN   MemRangeDescToken;
> +
> +  /// Unique identifier for this node.
> +  UINT32            Identifier;
The 'Identifier' field should be added above the 'Flags' as the
structure is a new and to respect the order of the spec.
> +} CM_ARM_RMR_NODE;
> +
> +/** A structure that describes the
> +    Memory Range descriptor.
> +
> +    ID: EArmObjMemoryRangeDescriptor
> +*/
> +typedef struct CmArmRmrDescriptor {
> +  /// Base address.
> +  UINT64            BaseAddress;
> +
> +  /// Length.
> +  UINT64            Length;

The exact names of the fields in the spec are:
-Physical Range offset
-Physical Range length

Would it be possible to add these names in the description ?
> +} CM_ARM_MEMORY_RANGE_DESCRIPTOR;
> +
>  #pragma pack()
>  
>  #endif // ARM_NAMESPACE_OBJECTS_H_

[PATCH v2 8/8] DynamicTablesPkg: IORT generator updates for Rev E.b spec

[Sami Mujawar]

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

The IO Remapping Table, Platform Design Document, Revision E.b,
Feb 2021 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E and Rev E.a:
  - increments the IORT table revision to 3.
  - 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.

Therefore, update the IORT generator to:
  - increment IORT table revision count to 3.
  - populate Identifier filed if revision is greater than 2.
  - add support to populate Reserved Memory Range nodes and
    the Memory range descriptors.
  - add validation to check that the Identifier field is
    unique.

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

Notes:
    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 | 661 ++++++++++++++++++--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h |   5 +-
 2 files changed, 624 insertions(+), 42 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index 9ccf72594db378878d4e3abbafe98e749d9963da..9b687f0165e6136f2f24749d27dee27edaff1b31 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.b, Feb 2021
+    (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.
 */
@@ -177,13 +197,16 @@ GetSizeofItsGroupNodes (
     (*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);
@@ -250,13 +273,16 @@ GetSizeofNamedComponentNodes (
     (*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);
@@ -322,13 +348,16 @@ GetSizeofRootComplexNodes (
     (*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);
@@ -400,13 +429,16 @@ GetSizeofSmmuV1V2Nodes (
     (*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);
@@ -471,13 +503,16 @@ GetSizeofSmmuV3Nodes (
     (*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,13 +577,16 @@ GetSizeofPmcgNodes (
     (*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);
@@ -558,6 +596,83 @@ 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.
@@ -707,6 +822,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.
@@ -723,6 +839,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,
@@ -759,11 +876,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_REV3) {
+      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 +
@@ -814,6 +937,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.
@@ -830,6 +954,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,
@@ -865,10 +990,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_REV3) {
+      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) +
@@ -899,8 +1030,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);
@@ -938,6 +1080,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.
@@ -954,6 +1097,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,
@@ -987,12 +1131,18 @@ 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_REV3) {
+      RcNode->Node.Revision = 1;
+      RcNode->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
+    } else {
+      RcNode->Node.Revision = 3;
+      RcNode->Node.Identifier = NodeList->Identifier;
+    }
+
     // Root Complex specific data
     RcNode->CacheCoherent = NodeList->CacheCoherent;
     RcNode->AllocationHints = NodeList->AllocationHints;
@@ -1005,8 +1155,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);
@@ -1107,6 +1268,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.
@@ -1123,6 +1285,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,
@@ -1159,8 +1322,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) +
@@ -1169,6 +1330,14 @@ AddSmmuV1V2Nodes (
            (NodeList->PmuInterruptCount *
            sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)));
 
+    if (AcpiTableInfo->AcpiTableRevision < EFI_ACPI_IO_REMAPPING_TABLE_REV3) {
+      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;
@@ -1263,8 +1432,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);
@@ -1300,6 +1480,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.
@@ -1314,6 +1495,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,
@@ -1346,12 +1528,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_REV3) {
+      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;
@@ -1379,8 +1567,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);
@@ -1417,6 +1616,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.
@@ -1431,6 +1631,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,
@@ -1465,12 +1666,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_REV3) {
+      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;
@@ -1494,8 +1701,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);
@@ -1526,6 +1744,273 @@ 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 = 1;
+    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
@@ -1570,6 +2055,7 @@ BuildIortTable (
   UINT32                                 SmmuV1V2NodeCount;
   UINT32                                 SmmuV3NodeCount;
   UINT32                                 PmcgNodeCount;
+  UINT32                                 RmrNodeCount;
 
   UINT32                                 ItsGroupOffset;
   UINT32                                 NamedComponentOffset;
@@ -1577,6 +2063,7 @@ BuildIortTable (
   UINT32                                 SmmuV1V2Offset;
   UINT32                                 SmmuV3Offset;
   UINT32                                 PmcgOffset;
+  UINT32                                 RmrOffset;
 
   CM_ARM_ITS_GROUP_NODE                * ItsGroupNodeList;
   CM_ARM_NAMED_COMPONENT_NODE          * NamedComponentNodeList;
@@ -1584,6 +2071,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;
@@ -1726,6 +2214,27 @@ BuildIortTable (
   // Add the PMCG node count
   IortNodeCount += PmcgNodeCount;
 
+  if (AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV3) {
+    // 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) *
@@ -1929,6 +2438,37 @@ BuildIortTable (
       ));
   }
 
+  // RMR Nodes
+  if ((AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV3) &&
+      (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" \
@@ -1950,6 +2490,19 @@ BuildIortTable (
     goto error_handler;
   }
 
+  // Validate that the identifiers for the nodes are unique
+  if (AcpiTableInfo->AcpiTableRevision >= EFI_ACPI_IO_REMAPPING_TABLE_REV3) {
+    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) {
@@ -1998,6 +2551,7 @@ BuildIortTable (
     Status = AddItsGroupNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                ItsGroupOffset,
                ItsGroupNodeList,
@@ -2017,6 +2571,7 @@ BuildIortTable (
     Status = AddNamedComponentNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                NamedComponentOffset,
                NamedComponentNodeList,
@@ -2036,6 +2591,7 @@ BuildIortTable (
     Status = AddRootComplexNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                RootComplexOffset,
                RootComplexNodeList,
@@ -2055,6 +2611,7 @@ BuildIortTable (
     Status = AddSmmuV1V2Nodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                SmmuV1V2Offset,
                SmmuV1V2NodeList,
@@ -2074,6 +2631,7 @@ BuildIortTable (
     Status = AddSmmuV3Nodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                SmmuV3Offset,
                SmmuV3NodeList,
@@ -2093,6 +2651,7 @@ BuildIortTable (
     Status = AddPmcgNodes (
                This,
                CfgMgrProtocol,
+               AcpiTableInfo,
                Iort,
                PmcgOffset,
                PmcgNodeList,
@@ -2101,7 +2660,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;
@@ -2184,11 +2763,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_REV3,
     // 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 bbf8aaf9efa38a75626a49af6f380b1c8c1a0629..afe2089b1dc20ed671583e2a309cc1a46c3404c6 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 - 2021, 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)'

Re: [edk2-devel] [PATCH v2 8/8] DynamicTablesPkg: IORT generator updates for Rev E.b spec

[PierreGondois]

Hi Sami,

With the small modification:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

On 6/17/21 10:55, Sami Mujawar via groups.io wrote:
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
>
> The IO Remapping Table, Platform Design Document, Revision E.b,
> Feb 2021 (https://developer.arm.com/documentation/den0049/)
> introduces the following updates, collectively including the
> updates and errata fixes to Rev E and Rev E.a:
>   - increments the IORT table revision to 3.
>   - 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.
>
> Therefore, update the IORT generator to:
>   - increment IORT table revision count to 3.
>   - populate Identifier filed if revision is greater than 2.
>   - add support to populate Reserved Memory Range nodes and
>     the Memory range descriptors.
>   - add validation to check that the Identifier field is
>     unique.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
>     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 | 661 ++++++++++++++++++--
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h |   5 +-
>  2 files changed, 624 insertions(+), 42 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> index 9ccf72594db378878d4e3abbafe98e749d9963da..9b687f0165e6136f2f24749d27dee27edaff1b31 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.b, Feb 2021
> +    (https://developer.arm.com/documentation/den0049/)
>  
>  **/
>  
[snip]
> +
>        // Ids for SMMUv3 node
>        IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE*)((UINT8*)SmmuV3Node +
>                      SmmuV3Node->Node.IdReference);
> @@ -1417,6 +1616,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.
> @@ -1431,6 +1631,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,
> @@ -1465,12 +1666,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_REV3) {
> +      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;
> @@ -1494,8 +1701,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;
> +      }
> +

As this is not related to the RevE spec update, could it be done in:

[PATCH v2 6/8] DynamicTablesPkg: IORT set reference to interrupt array
if present

This is also done at other places.

>        // Ids for PMCG node
>        IdMapArray = (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE*)((UINT8*)PmcgNode +
>                      PmcgNode->Node.IdReference);
> @@ -1526,6 +1744,273 @@ AddPmcgNodes (
>    return EFI_SUCCESS;
>  }
>  

[snip]

Regards,

Pierre

回复: [edk2-devel] [PATCH v2 0/8] IORT Rev E.b specification updates

[gaoliming]

Sami:
  I agree this change. With this patch, will you update the existing
platform to use the matched version macro EFI_ACPI_IO_REMAPPING_TABLE_REV0? 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sami
> Mujawar
> 发送时间: 2021年6月17日 17:55
> 收件人: devel@edk2.groups.io
> 抄送: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com;
> ardb+tianocore@kernel.org; Matteo.Carlini@arm.com;
> Ben.Adderson@arm.com; steven.price@arm.com;
> Lorenzo.Pieralisi@arm.com; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com; ray.ni@intel.com;
> zhichao.gao@intel.com; nd@arm.com
> 主题: [edk2-devel] [PATCH v2 0/8] IORT Rev E.b specification updates
> 
> Bugzilla: 3458 - Add support IORT Rev E.b specification updates
>           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
> 
> The IO Remapping Table (IORT) specification has been updated to
> rev E.b. The following updates are introduced including the errata
> to rev E and E.a:
>   - increments the IORT table revision to 3.
>   - 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.
> 
> 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.
> 
> This 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 changes can be seen at:
> https://github.com/samimujawar/edk2/tree/1527_iort_rev_eb_v2
> 
> Sami Mujawar (8):
>   MdePkg: Fix IORT header file include guard
>   MdePkg: IORT header update for IORT Rev E.b spec
>   ShellPkg: Acpiview: Abbreviate field names to preserve alignment
>   ShellPkg: Acpiview: IORT parser update for IORT Rev E.b spec
>   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.b
>   DynamicTablesPkg: IORT generator updates for Rev E.b spec
> 
>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> |  58 ++
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> | 772 ++++++++++++++++++--
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.h
> |   5 +-
>  MdePkg/Include/IndustryStandard/IoRemappingTable.h
> |  71 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |
> 207 +++++-
>  5 files changed, 1013 insertions(+), 100 deletions(-)
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 
>