Hi Joey,
Thanks for the review, I answered inline:


On 9/24/21 9:56 AM, Joey Gouly wrote:

Hi,

This looks good to me!

[...]

+
+/** A parser for EArmObjFixedFeatureFlags.
+*/
+STATIC CONST CM_OBJ_PARSER CmArmFixedFeatureFlagsParser[] = {
+ {"Flags", 4, "0x%x", NULL}
+};
+
+/** A parser for EArmObjItsGroup.
+*/
+STATIC CONST CM_OBJ_PARSER CmArmItsGroupNodeParser[] = {
+ {"GTBlockTimerFrameToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL},

This should just be Token, not GTBlockTimerFrameToken.

The name of the field is 'GTBlockTimerFrameToken', cf
https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/ArmNameSpaceObjects.h#L394
I am not sure I understand why this should 'Token' instead.

+ {"ItsIdCount", 4, "0x%x", NULL},
+ {"ItsIdToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL}
+};
+

[...]

diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.h b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.h
new file mode 100644
index 000000000000..e229df7095d9
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.h

[...]

+/**
+ The CM_OBJ_PARSER structure describes the fields of an CmObject and
+ provides means for the parser to interpret and trace appropriately.
+
+ ParseAcpi() uses the format string specified by 'Format' for tracing
+ the field data.
+*/
+typedef struct CmObjParser CM_OBJ_PARSER;
+struct CmObjParser {
+
+ /// String describing the Cm Object
+ CONST CHAR8* NameStr;
+
+ /// The length of the field.
+ UINT32 Length;
+
+ /// Optional Print() style format string for tracing the data. If not
+ /// used this must be set to NULL.
+ CONST CHAR8* Format;
+
+ /// Optional pointer to a print formatter function which
+ /// is typically used to trace complex field information.
+ /// If not used this must be set to NULL.
+ /// The Format string is passed to the PrintFormatter function
+ /// but may be ignored by the implementation code.
+ FNPTR_PRINT_FORMATTER PrintFormatter;
+
+ /// Optional pointer to print the fields of another CM_OBJ_PARSER
+ /// structure. This is useful to print sub-structures.
+ CONST CM_OBJ_PARSER *SubObjParser;
+
+ /// Count of items in the SubObj.
+ UINTN SubObjItemCount;

The SubObjParser doesn't actually seem to be used by any of the objects? (Unless I misread when reading the list of them..)

The SubObjParser field is effectively not currently used. It will be
used in a later patch,
cf the 'UsageCounterRegister' field of
https://edk2.groups.io/g/devel/message/76954

+
+ /// Count of items
+ UINTN ItemCount;
+} CM_OBJ_PARSER_ARRAY;
+
+#endif // CONFIGURATION_MANAGER_OBJECT_PARSER_H_
diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf b/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
index 5435f74aa0b8..abbf4bc38cab 100644
--- a/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
@@ -15,6 +15,8 @@ [Defines]
LIBRARY_CLASS = TableHelperLib

[Sources]
+ ConfigurationManagerObjectParser.c
+ ConfigurationManagerObjectParser.h
TableHelper.c

[Packages]

Need to update the copyright year.

Otherwise:
Reviewed-by: Joey Gouly <joey.gouly@...>

Thanks,
Joey

Regards,
Pierre