Bugzilla: 3458 - Add support IORT Rev E.d specification updates
(https://bugzilla.tianocore.org/show_bug.cgi?id=3458)
The IO Remapping Table, Platform Design Document, Revision E.d,
Feb 2022 (https://developer.arm.com/documentation/den0049/)
introduces the following updates, collectively including the
updates and errata fixes to Rev E, Rev E.a, Rev E.b, Rev E.c:
- increments the IORT table revision to 5.
- updates the node definition to add an 'Identifier' field.
- adds definition of node type 6 - Reserved Memory Range node.
- adds definition for Memory Range Descriptors.
- adds flag to indicate PRI support for root complexes.
- adds flag to indicate if the root complex supports forwarding
of PASID information on translated transactions to the SMMU.
- adds flag to indicate if the root complex supports PASID.
- adds flags to define access privilege and attributes for the
memory ranges.
Therefore, update the IORT parser to:
- parse the Identifier field.
- parse Reserved Memory Range node.
- parse Memory Range Descriptors.
- add validations to check that the physical range base
and size of the Memory Range Descriptor is 64KB aligned.
- add validation to check that the IORT Table Revision is
not 4 as IORT Rev E.c is deprecated.
- add validation to check that the IORT RMR node revision
is not 2 as it breaks backward compatibility and was
deprecated as part of IORT Rev E.c.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
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 | 221 ++++++++++++++++++--
1 file changed, 203 insertions(+), 18 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 44d633c5282463078a4cc990bb24ca1992f95634..0b42ff3b354a7cf54cdc2e6e15d0617fadda17f2 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -1,14 +1,16 @@
/** @file
IORT table parser
- Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+ Copyright (c) 2016 - 2022, Arm Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
- - IO Remapping Table, Platform Design Document, Revision D, March 2018
+ - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022
+ (https://developer.arm.com/documentation/den0049/)
@par Glossary:
- Ref - Reference
+ - Desc - Descriptor
**/
#include <IndustryStandard/IoRemappingTable.h>
@@ -26,6 +28,7 @@ STATIC CONST UINT32 *IortNodeOffset;
STATIC CONST UINT8 *IortNodeType;
STATIC CONST UINT16 *IortNodeLength;
+STATIC CONST UINT8 *IortNodeRevision;
STATIC CONST UINT32 *IortIdMappingCount;
STATIC CONST UINT32 *IortIdMappingOffset;
@@ -36,6 +39,9 @@ STATIC CONST UINT32 *PmuInterruptOffset;
STATIC CONST UINT32 *ItsCount;
+STATIC CONST UINT32 *RmrMemDescCount;
+STATIC CONST UINT32 *RmrMemDescOffset;
+
/**
This function validates the ID Mapping array count for the ITS node.
@@ -100,6 +106,52 @@ ValidateItsIdArrayReference (
}
}
+/**
+ This function validates that the Physical Range address or length is not zero
+ and is 64K aligned.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidatePhysicalRange (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ UINT64 Value;
+
+ Value = *(UINT64 *)Ptr;
+ if ((Value == 0) || ((Value & (SIZE_64KB - 1)) != 0)) {
+ IncrementErrorCount ();
+ Print (L"\nERROR: Physical Range must be 64K aligned and cannot be zero.");
+ }
+}
+
+/**
+ This function validates that the RMR memory range descriptor count.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateRmrMemDescCount (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ if (*(UINT32 *)Ptr == 0) {
+ IncrementErrorCount ();
+ Print (L"\nERROR: Memory Range Descriptor count must be >=1.");
+ }
+}
+
/**
Helper Macro for populating the IORT Node header in the ACPI_PARSER array.
@@ -108,15 +160,15 @@ ValidateItsIdArrayReference (
@param [out] ValidateIdArrayReference Optional pointer to a function for
validating the ID Array reference.
**/
-#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
- ValidateIdArrayReference) \
- { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \
- { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \
- { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL }, \
- { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \
- { L"Number of ID mappings", 4, 8, L"%d", NULL, \
- (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \
- { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \
+#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
+ ValidateIdArrayReference) \
+ { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \
+ { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \
+ { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL, NULL }, \
+ { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \
+ { L"Number of ID mappings", 4, 8, L"%d", NULL, \
+ (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \
+ { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \
(VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL }
/**
@@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER IortNodeNamedComponentParser[] = {
**/
STATIC CONST ACPI_PARSER IortNodeRootComplexParser[] = {
PARSE_IORT_NODE_HEADER (NULL, NULL),
- { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL },
- { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL },
- { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL }
+ { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL },
+ { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"PASID capabilities", 2, 33, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Reserved", 1, 35, L"%x", NULL, NULL, NULL, NULL },
+ { L"Flags", 4, 36, L"%x", NULL, NULL, NULL, NULL },
};
/**
@@ -253,6 +307,29 @@ STATIC CONST ACPI_PARSER IortNodePmcgParser[] = {
{ L"Page 1 Base Address", 8, 32, L"0x%lx", NULL, NULL, NULL, NULL }
};
+/**
+ An ACPI_PARSER array describing the IORT RMR node.
+**/
+STATIC CONST ACPI_PARSER IortNodeRmrParser[] = {
+ PARSE_IORT_NODE_HEADER (NULL, NULL),
+ { L"Flags", 4, 16, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Memory Range Desc count", 4, 20, L"%d", NULL,
+ (VOID **)&RmrMemDescCount, ValidateRmrMemDescCount,NULL },
+ { L"Memory Range Desc Ref", 4, 24, L"0x%x", NULL,
+ (VOID **)&RmrMemDescOffset, NULL, NULL }
+};
+
+/**
+ An ACPI_PARSER array describing the IORT RMR Memory Range Descriptor.
+**/
+STATIC CONST ACPI_PARSER IortNodeRmrMemRangeDescParser[] = {
+ { L"Physical Range offset", 8, 0, L"0x%lx", NULL, NULL, ValidatePhysicalRange,
+ NULL },
+ { L"Physical Range length", 8, 8, L"0x%lx", NULL, NULL, ValidatePhysicalRange,
+ NULL },
+ { L"Reserved", 4, 16, L"0x%x", NULL, NULL, NULL, NULL}
+};
+
/**
This function parses the IORT Node Id Mapping array.
@@ -607,9 +684,102 @@ DumpIortNodePmcg (
);
}
+/**
+ This function parses the IORT RMR Node Memory Range Descriptor array.
+
+ @param [in] Ptr Pointer to the start of the Memory Range Descriptor
+ array.
+ @param [in] Length Length of the buffer.
+ @param [in] DescCount Memory Range Descriptor count.
+**/
+STATIC
+VOID
+DumpIortNodeRmrMemRangeDesc (
+ IN UINT8 *Ptr,
+ IN UINT32 Length,
+ IN UINT32 DescCount
+ )
+{
+ UINT32 Index;
+ UINT32 Offset;
+ CHAR8 Buffer[40]; // Used for AsciiName param of ParseAcpi
+
+ Index = 0;
+ Offset = 0;
+
+ while ((Index < DescCount) &&
+ (Offset < Length))
+ {
+ AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "Mem range Descriptor [%d]",
+ Index
+ );
+ Offset += ParseAcpi (
+ TRUE,
+ 4,
+ Buffer,
+ Ptr + Offset,
+ Length - Offset,
+ PARSER_PARAMS (IortNodeRmrMemRangeDescParser)
+ );
+ Index++;
+ }
+}
+
+/**
+ This function parses the IORT RMR node.
+
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the buffer.
+ @param [in] MappingCount The ID Mapping count.
+ @param [in] MappingOffset The offset of the ID Mapping array
+ from the start of the IORT table.
+**/
+STATIC
+VOID
+DumpIortNodeRmr (
+ IN UINT8 *Ptr,
+ IN UINT16 Length,
+ IN UINT32 MappingCount,
+ IN UINT32 MappingOffset
+ )
+{
+ ParseAcpi (
+ TRUE,
+ 2,
+ "RMR Node",
+ Ptr,
+ Length,
+ PARSER_PARAMS (IortNodeRmrParser)
+ );
+
+ if (*IortNodeRevision == 2) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: RMR node Rev 2 (defined in IORT Rev E.c) must not be used."
+ L" IORT tabe Revision E.c is deprecated and must not be used.\n"
+ );
+ }
+
+ DumpIortNodeIdMappings (
+ Ptr + MappingOffset,
+ Length - MappingOffset,
+ MappingCount
+ );
+
+ DumpIortNodeRmrMemRangeDesc (
+ Ptr + (*RmrMemDescOffset),
+ Length - (*RmrMemDescOffset),
+ *RmrMemDescCount
+ );
+}
+
/**
This function parses the ACPI IORT table.
- When trace is enabled this function parses the IORT table and traces the ACPI fields.
+ When trace is enabled this function parses the IORT table and traces the ACPI
+ fields.
This function also parses the following nodes:
- ITS Group
@@ -618,6 +788,7 @@ DumpIortNodePmcg (
- SMMUv1/2
- SMMUv3
- PMCG
+ - RMR
This function also performs validation of the ACPI table fields.
@@ -643,6 +814,13 @@ ParseAcpiIort (
return;
}
+ if (AcpiTableRevision == 4) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: IORT tabe Revision E.c is deprecated and must not be used.\n"
+ );
+ }
+
ParseAcpi (
TRUE,
0,
@@ -765,7 +943,14 @@ ParseAcpiIort (
*IortIdMappingOffset
);
break;
-
+ case EFI_ACPI_IORT_TYPE_RMR:
+ DumpIortNodeRmr (
+ NodePtr,
+ *IortNodeLength,
+ *IortIdMappingCount,
+ *IortIdMappingOffset
+ );
+ break;
default:
IncrementErrorCount ();
Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91265): https://edk2.groups.io/g/devel/message/91265
Mute This Topic: https://groups.io/mt/92334076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi Sami,
> +
> /**
> 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 }
Sorry to come back on this patch, but it seems the identifier field was
16 bits long in rev E (=1) and increased to 32 bits in rev E.a (=2),
so unfortunately, an extra case should be created for this I believe.
Regards,
Pierre
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91310): https://edk2.groups.io/g/devel/message/91310
Mute This Topic: https://groups.io/mt/92334076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Sami,
> +
> /**
> Helper Macro for populating the IORT Node header in the ACPI_PARSER array.
>
> @@ -108,15 +160,15 @@ ValidateItsIdArrayReference (
> @param [out] ValidateIdArrayReference Optional pointer to a function for
> validating the ID Array reference.
> **/
> -#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
> - ValidateIdArrayReference) \
> - { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \
> - { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \
> - { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL }, \
> - { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \
> - { L"Number of ID mappings", 4, 8, L"%d", NULL, \
> - (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \
> - { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \
> +#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
> + ValidateIdArrayReference) \
> + { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL }, \
> + { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL, NULL }, \
> + { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL, NULL }, \
> + { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, \
> + { L"Number of ID mappings", 4, 8, L"%d", NULL, \
> + (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL }, \
> + { L"Reference to ID Array", 4, 12, L"0x%x", NULL, \
> (VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL }
>
> /**
> @@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER IortNodeNamedComponentParser[] = {
> **/
> STATIC CONST ACPI_PARSER IortNodeRootComplexParser[] = {
> PARSE_IORT_NODE_HEADER (NULL, NULL),
> - { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL },
> - { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL },
> - { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL },
> - { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL },
> - { L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL }
> + { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL, NULL, NULL },
> + { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL, NULL, NULL },
> + { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL, NULL, NULL },
> + { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL, NULL, NULL },
> + { L"PASID capabilities", 2, 33, L"0x%x", NULL, NULL, NULL, NULL },
> + { L"Reserved", 1, 35, L"%x", NULL, NULL, NULL, NULL },
> + { L"Flags", 4, 36, L"%x", NULL, NULL, NULL, NULL },
It seems flags are usually printed as L"0x%x"
> };
>
> /**
> @@ -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}
> +};
> +
> /**
[snip]
> /**
> This function parses the ACPI IORT table.
> - When trace is enabled this function parses the IORT table and traces the ACPI fields.
> + When trace is enabled this function parses the IORT table and traces the ACPI
> + fields.
>
> This function also parses the following nodes:
> - ITS Group
> @@ -618,6 +788,7 @@ DumpIortNodePmcg (
> - SMMUv1/2
> - SMMUv3
> - PMCG
> + - RMR
>
> This function also performs validation of the ACPI table fields.
>
> @@ -643,6 +814,13 @@ ParseAcpiIort (
> return;
> }
>
> + if (AcpiTableRevision == 4) {
> + IncrementErrorCount ();
> + Print (
> + L"ERROR: IORT tabe Revision E.c is deprecated and must not be used.\n"
> + );
> + }
> +
I think a macro corresponding to rev E.c should be added in MdePkg
so we can identify the deprecated revision easily.
Similar comment for the Rmr revision (=2) which is checked in DumpIortNodeRmr().
> ParseAcpi (
> TRUE,
> 0,
> @@ -765,7 +943,14 @@ ParseAcpiIort (
> *IortIdMappingOffset
> );
> break;
> -
> + case EFI_ACPI_IORT_TYPE_RMR:
> + DumpIortNodeRmr (
> + NodePtr,
> + *IortNodeLength,
> + *IortIdMappingCount,
> + *IortIdMappingOffset
> + );
> + break;
> default:
> IncrementErrorCount ();
> Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91308): https://edk2.groups.io/g/devel/message/91308
Mute This Topic: https://groups.io/mt/92334076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Pierre,
Thank you for the feedback.
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 13/07/2022 01:10 pm, Pierre Gondois wrote:
> Hi Sami,
>
>> +
>> /**
>> Helper Macro for populating the IORT Node header in the
>> ACPI_PARSER array.
>> @@ -108,15 +160,15 @@ ValidateItsIdArrayReference (
>> @param [out] ValidateIdArrayReference Optional pointer to a
>> function for
>> validating the ID Array
>> reference.
>> **/
>> -#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
>> - ValidateIdArrayReference) \
>> - { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL
>> }, \
>> - { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL,
>> NULL }, \
>> - { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL
>> }, \
>> - { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL
>> }, \
>> - { L"Number of ID mappings", 4, 8, L"%d",
>> NULL, \
>> - (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL
>> }, \
>> - { L"Reference to ID Array", 4, 12, L"0x%x",
>> NULL, \
>> +#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
>> + ValidateIdArrayReference) \
>> + { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL
>> }, \
>> + { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL,
>> NULL }, \
>> + { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL,
>> NULL }, \
>> + { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL
>> }, \
>> + { L"Number of ID mappings", 4, 8, L"%d",
>> NULL, \
>> + (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL
>> }, \
>> + { L"Reference to ID Array", 4, 12, L"0x%x",
>> NULL, \
>> (VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL }
>> /**
>> @@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER
>> IortNodeNamedComponentParser[] = {
>> **/
>> STATIC CONST ACPI_PARSER IortNodeRootComplexParser[] = {
>> PARSE_IORT_NODE_HEADER (NULL, NULL),
>> - { L"Memory access properties",8, 16, L"0x%lx", NULL,
>> NULL, NULL, NULL },
>> - { L"ATS Attribute", 4, 24, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"PCI Segment number", 4, 28, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"Memory access size limit",1, 32, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"Reserved", 3, 33, L"%x %x %x", Dump3Chars,
>> NULL, NULL, NULL }
>> + { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL,
>> NULL, NULL },
>> + { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"PASID capabilities", 2, 33, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Reserved", 1, 35, L"%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Flags", 4, 36, L"%x", NULL, NULL,
>> NULL, NULL },
>
> It seems flags are usually printed as L"0x%x"
[SAMI] Ack.
>
>> };
>> /**
>> @@ -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}
>> +};
>> +
>> /**
>
> [snip]
>
>> /**
>> This function parses the ACPI IORT table.
>> - When trace is enabled this function parses the IORT table and
>> traces the ACPI fields.
>> + When trace is enabled this function parses the IORT table and
>> traces the ACPI
>> + fields.
>> This function also parses the following nodes:
>> - ITS Group
>> @@ -618,6 +788,7 @@ DumpIortNodePmcg (
>> - SMMUv1/2
>> - SMMUv3
>> - PMCG
>> + - RMR
>> This function also performs validation of the ACPI table fields.
>> @@ -643,6 +814,13 @@ ParseAcpiIort (
>> return;
>> }
>> + if (AcpiTableRevision == 4) {
>> + IncrementErrorCount ();
>> + Print (
>> + L"ERROR: IORT tabe Revision E.c is deprecated and must not be
>> used.\n"
>> + );
>> + }
>> +
>
> I think a macro corresponding to rev E.c should be added in MdePkg
> so we can identify the deprecated revision easily.
> Similar comment for the Rmr revision (=2) which is checked in
> DumpIortNodeRmr().
>
[SAMI] Ack.
>
>> ParseAcpi (
>> TRUE,
>> 0,
>> @@ -765,7 +943,14 @@ ParseAcpiIort (
>> *IortIdMappingOffset
>> );
>> break;
>> -
>> + case EFI_ACPI_IORT_TYPE_RMR:
>> + DumpIortNodeRmr (
>> + NodePtr,
>> + *IortNodeLength,
>> + *IortIdMappingCount,
>> + *IortIdMappingOffset
>> + );
>> + break;
>> default:
>> IncrementErrorCount ();
>> Print (L"ERROR: Unsupported IORT Node type = %d\n",
>> *IortNodeType);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91317): https://edk2.groups.io/g/devel/message/91317
Mute This Topic: https://groups.io/mt/92334076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Pierre,
Thank you for the feedback.
Please find my response inline marked [SAMI]
On 13/07/2022 01:10 pm, Pierre Gondois wrote:
> Hi Sami,
>
>> +
>> /**
>> Helper Macro for populating the IORT Node header in the
>> ACPI_PARSER array.
>> @@ -108,15 +160,15 @@ ValidateItsIdArrayReference (
>> @param [out] ValidateIdArrayReference Optional pointer to a
>> function for
>> validating the ID Array
>> reference.
>> **/
>> -#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
>> - ValidateIdArrayReference) \
>> - { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL
>> }, \
>> - { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL,
>> NULL }, \
>> - { L"Revision", 1, 3, L"%d", NULL, NULL, NULL, NULL
>> }, \
>> - { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL
>> }, \
>> - { L"Number of ID mappings", 4, 8, L"%d",
>> NULL, \
>> - (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL
>> }, \
>> - { L"Reference to ID Array", 4, 12, L"0x%x",
>> NULL, \
>> +#define PARSE_IORT_NODE_HEADER(ValidateIdMappingCount, \
>> + ValidateIdArrayReference) \
>> + { L"Type", 1, 0, L"%d", NULL, (VOID**)&IortNodeType, NULL, NULL
>> }, \
>> + { L"Length", 2, 1, L"%d", NULL, (VOID**)&IortNodeLength, NULL,
>> NULL }, \
>> + { L"Revision", 1, 3, L"%d", NULL, (VOID**)&IortNodeRevision, NULL,
>> NULL }, \
>> + { L"Identifier", 4, 4, L"0x%x", NULL, NULL, NULL, NULL
>> }, \
>> + { L"Number of ID mappings", 4, 8, L"%d",
>> NULL, \
>> + (VOID**)&IortIdMappingCount, ValidateIdMappingCount, NULL
>> }, \
>> + { L"Reference to ID Array", 4, 12, L"0x%x",
>> NULL, \
>> (VOID**)&IortIdMappingOffset, ValidateIdArrayReference, NULL }
>> /**
>> @@ -235,11 +287,13 @@ STATIC CONST ACPI_PARSER
>> IortNodeNamedComponentParser[] = {
>> **/
>> STATIC CONST ACPI_PARSER IortNodeRootComplexParser[] = {
>> PARSE_IORT_NODE_HEADER (NULL, NULL),
>> - { L"Memory access properties",8, 16, L"0x%lx", NULL,
>> NULL, NULL, NULL },
>> - { L"ATS Attribute", 4, 24, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"PCI Segment number", 4, 28, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"Memory access size limit",1, 32, L"0x%x", NULL,
>> NULL, NULL, NULL },
>> - { L"Reserved", 3, 33, L"%x %x %x", Dump3Chars,
>> NULL, NULL, NULL }
>> + { L"Memory access properties",8, 16, L"0x%lx", NULL, NULL,
>> NULL, NULL },
>> + { L"ATS Attribute", 4, 24, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"PCI Segment number", 4, 28, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Memory access size limit",1, 32, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"PASID capabilities", 2, 33, L"0x%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Reserved", 1, 35, L"%x", NULL, NULL,
>> NULL, NULL },
>> + { L"Flags", 4, 36, L"%x", NULL, NULL,
>> NULL, NULL },
>
> It seems flags are usually printed as L"0x%x"
[SAMI] Ack.
>
>> };
>> /**
>> @@ -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}
>> +};
>> +
>> /**
>
> [snip]
>
>> /**
>> This function parses the ACPI IORT table.
>> - When trace is enabled this function parses the IORT table and
>> traces the ACPI fields.
>> + When trace is enabled this function parses the IORT table and
>> traces the ACPI
>> + fields.
>> This function also parses the following nodes:
>> - ITS Group
>> @@ -618,6 +788,7 @@ DumpIortNodePmcg (
>> - SMMUv1/2
>> - SMMUv3
>> - PMCG
>> + - RMR
>> This function also performs validation of the ACPI table fields.
>> @@ -643,6 +814,13 @@ ParseAcpiIort (
>> return;
>> }
>> + if (AcpiTableRevision == 4) {
>> + IncrementErrorCount ();
>> + Print (
>> + L"ERROR: IORT tabe Revision E.c is deprecated and must not be
>> used.\n"
>> + );
>> + }
>> +
>
> I think a macro corresponding to rev E.c should be added in MdePkg
> so we can identify the deprecated revision easily.
> Similar comment for the Rmr revision (=2) which is checked in
> DumpIortNodeRmr().
>
[SAMI] Ack.
>
>> ParseAcpi (
>> TRUE,
>> 0,
>> @@ -765,7 +943,14 @@ ParseAcpiIort (
>> *IortIdMappingOffset
>> );
>> break;
>> -
>> + case EFI_ACPI_IORT_TYPE_RMR:
>> + DumpIortNodeRmr (
>> + NodePtr,
>> + *IortNodeLength,
>> + *IortIdMappingCount,
>> + *IortIdMappingOffset
>> + );
>> + break;
>> default:
>> IncrementErrorCount ();
>> Print (L"ERROR: Unsupported IORT Node type = %d\n",
>> *IortNodeType);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91316): https://edk2.groups.io/g/devel/message/91316
Mute This Topic: https://groups.io/mt/92334076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.