From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web08.4416.1666951544336953725 for ; Fri, 28 Oct 2022 03:05:44 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 44E401FB; Fri, 28 Oct 2022 03:05:50 -0700 (PDT) Received: from [10.57.2.46] (unknown [10.57.2.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0F33F3F71A; Fri, 28 Oct 2022 03:05:42 -0700 (PDT) Message-ID: <642b3a15-7695-a41b-4a9b-2de482cacb7d@arm.com> Date: Fri, 28 Oct 2022 12:05:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH 03/14] DynamicTablesPkg: Update CmObjParser for IORT Rev E.d To: Sami Mujawar , devel@edk2.groups.io Cc: Alexei Fedorov , "nd@arm.com" References: <20221010092058.118714-1-Pierre.Gondois@arm.com> <20221010092058.118714-4-Pierre.Gondois@arm.com> <22e077ba-0067-f56a-689b-861305056e12@arm.com> From: "PierreGondois" In-Reply-To: <22e077ba-0067-f56a-689b-861305056e12@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Sami, On 10/26/22 14:34, Sami Mujawar wrote: > Hi Pierre, > > I have one comment marked inline as [SAMI]. > > Other than that change this patch should be good. Yes indeed, thanks for spotting it and for making the modification, Regards, Pierre > > Regards, > > Sami Mujawar > > On 10/10/2022 10:20 am, Pierre.Gondois@arm.com wrote: >> From: Pierre Gondois >> >> commit de200b7e2c3c ("DynamicTablesPkg: Update ArmNameSpaceObjects for >> IORT Rev E.d") >> adds new CmObj structures and fields to the ArmNameSpaceObjects. >> Update the CmObjectParser accordingly. >> >> Signed-off-by: Pierre Gondois >> --- >> .../ConfigurationManagerObjectParser.c | 59 ++++++++++++++----- >> 1 file changed, 45 insertions(+), 14 deletions(-) >> >> diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c >> index b46f19693bb5..80ebb0708661 100644 >> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c >> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c >> @@ -183,21 +183,23 @@ STATIC CONST CM_OBJ_PARSER CmArmFixedFeatureFlagsParser[] = { >> STATIC CONST CM_OBJ_PARSER CmArmItsGroupNodeParser[] = { >> { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> { "ItsIdCount", 4, "0x%x", NULL }, >> - { "ItsIdToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL } >> + { "ItsIdToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjNamedComponent. >> */ >> STATIC CONST CM_OBJ_PARSER CmArmNamedComponentNodeParser[] = { >> - { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> - { "IdMappingCount", 4, "0x%x", NULL }, >> - { "IdMappingToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> - { "Flags", 4, "0x%x", NULL }, >> - { "CacheCoherent", 4, "0x%x", NULL }, >> - { "AllocationHints", 1, "0x%x", NULL }, >> - { "MemoryAccessFlags", 1, "0x%x", NULL }, >> - { "AddressSizeLimit", 1, "0x%x", NULL }, >> - { "ObjectName", sizeof (CHAR8 *), "%a", NULL } >> + { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "IdMappingCount", 4, "0x%x", NULL }, >> + { "IdMappingToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "Flags", 4, "0x%x", NULL }, >> + { "CacheCoherent", 4, "0x%x", NULL }, >> + { "AllocationHints", 1, "0x%x", NULL }, >> + { "MemoryAccessFlags", 1, "0x%x", NULL }, >> + { "AddressSizeLimit", 1, "0x%x", NULL }, >> + { "ObjectName", 1, NULL, PrintString }, > > [SAMI] I think the Length field for ObjectName should be "sizeof (CHAR8 > *)" otherwise PrintCmObjDesc() would not advance to the next field > correctly/ > > If you agree, I will make this change locally before pushing the patch. > > [/SAMI] > >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjRootComplex. >> @@ -211,7 +213,10 @@ STATIC CONST CM_OBJ_PARSER CmArmRootComplexNodeParser[] = { >> { "MemoryAccessFlags", 1, "0x%x", NULL }, >> { "AtsAttribute", 4, "0x%x", NULL }, >> { "PciSegmentNumber", 4, "0x%x", NULL }, >> - { "MemoryAddressSize", 1, "0x%x", NULL } >> + { "MemoryAddressSize", 1, "0x%x", NULL }, >> + { "PasidCapabilities", 2, "0x%x", NULL }, >> + { "Flags", 4, "0x%x", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjSmmuV1SmmuV2. >> @@ -231,7 +236,8 @@ STATIC CONST CM_OBJ_PARSER CmArmSmmuV1SmmuV2NodeParser[] = { >> { "SMMU_NSgIrpt", 4, "0x%x", NULL }, >> { "SMMU_NSgIrptFlags", 4, "0x%x", NULL }, >> { "SMMU_NSgCfgIrpt", 4, "0x%x", NULL }, >> - { "SMMU_NSgCfgIrptFlags", 4, "0x%x", NULL } >> + { "SMMU_NSgCfgIrptFlags", 4, "0x%x", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjSmmuV3. >> @@ -249,7 +255,8 @@ STATIC CONST CM_OBJ_PARSER CmArmSmmuV3NodeParser[] = { >> { "GerrInterrupt", 4, "0x%x", NULL }, >> { "SyncInterrupt", 4, "0x%x", NULL }, >> { "ProximityDomain", 4, "0x%x", NULL }, >> - { "DeviceIdMappingIndex", 4, "0x%x", NULL } >> + { "DeviceIdMappingIndex", 4, "0x%x", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjPmcg. >> @@ -261,7 +268,8 @@ STATIC CONST CM_OBJ_PARSER CmArmPmcgNodeParser[] = { >> { "BaseAddress", 8, "0x%llx", NULL }, >> { "OverflowInterrupt", 4, "0x%x", NULL }, >> { "Page1BaseAddress", 8, "0x%llx", NULL }, >> - { "ReferenceToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL } >> + { "ReferenceToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> }; >> >> /** A parser for EArmObjGicItsIdentifierArray. >> @@ -432,6 +440,25 @@ STATIC CONST CM_OBJ_PARSER CmPciInterruptMapInfoParser[] = { >> ARRAY_SIZE (CmArmGenericInterruptParser) }, >> }; >> >> +/** A parser for EArmObjRmr. >> +*/ >> +STATIC CONST CM_OBJ_PARSER CmArmRmrInfoParser[] = { >> + { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "IdMappingCount", 4, "0x%x", NULL }, >> + { "IdMappingToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> + { "Identifier", 4, "0x%x", NULL }, >> + { "Flags", 4, "0x%x", NULL }, >> + { "MemRangeDescCount", 4, "0x%x", NULL }, >> + { "MemRangeDescToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }, >> +}; >> + >> +/** A parser for EArmObjMemoryRangeDescriptor. >> +*/ >> +STATIC CONST CM_OBJ_PARSER CmArmMemoryRangeDescriptorInfoParser[] = { >> + { "BaseAddress", 8, "0x%llx", NULL }, >> + { "Length", 8, "0x%llx", NULL }, >> +}; >> + >> /** A parser for EArmObjCpcInfo. >> */ >> STATIC CONST CM_OBJ_PARSER CmArmCpcInfoParser[] = { >> @@ -588,6 +615,10 @@ STATIC CONST CM_OBJ_PARSER_ARRAY ArmNamespaceObjectParser[] = { >> ARRAY_SIZE (CmArmPciAddressMapInfoParser) }, >> { "EArmObjPciInterruptMapInfo", CmPciInterruptMapInfoParser, >> ARRAY_SIZE (CmPciInterruptMapInfoParser) }, >> + { "EArmObjRmr", CmArmRmrInfoParser, >> + ARRAY_SIZE (CmArmRmrInfoParser) }, >> + { "EArmObjMemoryRangeDescriptor", CmArmMemoryRangeDescriptorInfoParser, >> + ARRAY_SIZE (CmArmMemoryRangeDescriptorInfoParser) }, >> { "EArmObjCpcInfo", CmArmCpcInfoParser, >> ARRAY_SIZE (CmArmCpcInfoParser) }, >> { "EArmObjMax", NULL, 0 },