Re: [edk2-devel] [PATCH v6 6/8] ShellPkg: Acpiview: IORT parser update for IORT Rev E.d spec

["Gao, Zhichao" <zhichao.gao@intel.com>]

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

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami
> Mujawar
> Sent: Friday, July 15, 2022 12:50 AM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com;
> pierre.gondois@arm.com; steven.price@arm.com;
> Lorenzo.Pieralisi@arm.com; Matteo.Carlini@arm.com;
> Akanksha.Jain2@arm.com; Ben.Adderson@arm.com; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; nd@arm.com
> Subject: [edk2-devel] [PATCH v6 6/8] ShellPkg: Acpiview: IORT parser update
> for IORT Rev E.d spec
> 
> 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.mujawar@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..599cf0ee8f7e4c7ed398fa6046
> 02604d1955d2e6 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortPars
> +++ er.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);
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 
>