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.web10.5887.1662978901527857478 for ; Mon, 12 Sep 2022 03:35:01 -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 48EC3113E; Mon, 12 Sep 2022 03:35:07 -0700 (PDT) Received: from [192.168.1.11] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A87A93F71A; Mon, 12 Sep 2022 03:34:59 -0700 (PDT) Message-ID: Date: Mon, 12 Sep 2022 12:34:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [edk2-devel] [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object To: devel@edk2.groups.io, jbrasen@nvidia.com Cc: ardb+tianocore@kernel.org, Sami.Mujawar@arm.com, Alexei.Fedorov@arm.com References: <99283b0ac5e1a9306e28afd2a76059803e2e7376.1662563529.git.jbrasen@nvidia.com> From: "PierreGondois" In-Reply-To: <99283b0ac5e1a9306e28afd2a76059803e2e7376.1662563529.git.jbrasen@nvidia.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Jeff, Please find some remarks inline: On 9/7/22 17:15, Jeff Brasen via groups.io wrote: > Introduce the CM_ARM_CPC_INFO CmObj in the ArmNameSpaceObjects. > > This allows to describe CPC information, as described in ACPI 6.4, > > s8.4.7.1 "_CPC (Continuous Performance Control)". > > > > Signed-off-by: Jeff Brasen > > --- > > .../Include/ArmNameSpaceObjects.h | 146 ++++++++++++++++-- > > .../ConfigurationManagerObjectParser.c | 79 ++++++++++ > > 2 files changed, 208 insertions(+), 17 deletions(-) > > > > diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > > index 102e0f96be..4d3f9ae534 100644 > > --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > > +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > > @@ -63,6 +63,7 @@ typedef enum ArmObjectID { > > EArmObjPciInterruptMapInfo, ///< 39 - Pci Interrupt Map Info > > EArmObjRmr, ///< 40 - Reserved Memory Range Node > > EArmObjMemoryRangeDescriptor, ///< 41 - Memory Range Descriptor > > + EArmObjCpcInfo, ///< 42 - Continuous Performance Control Info > > EArmObjMax > > } EARM_OBJECT_ID; > > [snip] > > +/** A structure that describes the Cpc information. > > + > > + Continuous Performance Control is described in DSDT/SSDT and associated > > + to cpus/clusters in the cpu topology. > > + > > + Unsupported Optional registers should be encoded with NULL resource > > + Register {(SystemMemory, 0, 0, 0, 0)} > > + > > + For values that support Integer or Buffer, integer will be used > > + if buffer is NULL resource. > > + If resource is not NULL then Integer must be 0 > > + > > + Cf. ACPI 6.4, s8.4.7.1 _CPC (Continuous Performance Control) > > + > > + ID: EArmObjCpcInfo > > +*/ > > +typedef struct CmArmCpcInfo { I think it would be good to have the CPC Revision entry here aswell. The NumEntries can be inferred from the Revision. > > + /// Indicates the highest level of performance the processor > > + /// is theoretically capable of achieving. > > + EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE HighestPerformanceBuffer; I think we can use ACPI 6.4 structs (it should be the same). This comment can be applied to all the *6_3* objects. > > + UINT32 HighestPerformanceInteger; > > + > [snip] > > @@ -501,6 +578,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY ArmNamespaceObjectParser[] = { > > ARRAY_SIZE (CmArmPciAddressMapInfoParser) }, > > { "EArmObjPciInterruptMapInfo", CmPciInterruptMapInfoParser, > > ARRAY_SIZE (CmPciInterruptMapInfoParser) }, > (for Sami) Not related to this patchset, but the following parsers are missing: -EArmObjRmr -EArmObjMemoryRangeDescriptor > + { "EArmObjCpcInfo", CmArmCpcInfoParser, > > + ARRAY_SIZE (CmArmCpcInfoParser) }, > > { "EArmObjMax", NULL, 0 }, > > }; > > >