Hi Zhichao,

Is it possible to provide your feedback for this patch, please?

Regards,

Sami Mujawar

On 14/07/2022 05:50 pm, Sami Mujawar wrote:
Bugzilla: 3458 - Add support IORT Rev E.d specification updates
           (https://bugzilla.tianocore.org/show_bug.cgi?id=3458)

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

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

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

Notes:
     v6:
      - Make output format for Flags filed consistent.           [PIERRE]
      - Define macro for IORT Rev E.c and RMR Node revision 2    [PIERRE]
        and use these to check for deprecated table/node
        revisions.
      - Identifier field size changed between IORT revision E    [PIERRE]
        and E.a. so extra checks should be needed.
      - Added check to skip parsing if IORT Rev E, E.<a,b,c>     [SAMI]
        as various fields were deprecated between revisions.
v5:
      - No code change since v4. Included r-b received for       [SAMI]
        v5 series.
v4:
      - Add validation to check that the IORT Table Revision is  [SAMI]
        not 4 as IORT Rev E.c is deprecated.
      - Add validation to check that the IORT RMR node revision  [SAMI]
        is not 2 as it breaks backward compatibility and was
        deprecated as part of IORT Rev E.c.
v3:
      - No code change since v1. Included r-b received for       [SAMI]
        v2 series.
        Ref: https://edk2.groups.io/g/devel/topic/83600720#7665
v2:
       - No code change since v1. Re-sending with v2 series.     [SAMI]

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

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91442): https://edk2.groups.io/g/devel/message/91442
Mute This Topic: https://groups.io/mt/92383465/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to