From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Jeff Brasen <jbrasen@nvidia.com>, devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org, Alexei.Fedorov@arm.com,
pierre.gondois@arm.com, "nd@arm.com" <nd@arm.com>
Subject: Re: [PATCH v3 3/3] DynamicTablesPkg: SSDT CPU _CPC generator
Date: Thu, 15 Sep 2022 16:16:35 +0100 [thread overview]
Message-ID: <3b4de5f5-fece-cab1-e27b-d07888390a6c@arm.com> (raw)
In-Reply-To: <bc1e95b7943057c24fe66433a664a1436737d0b5.1663247185.git.jbrasen@nvidia.com>
Hi Jeff,
Please find my inputs inline marked [SAMI].
Regards,
Sami Mujawar
On 15/09/2022 02:10 pm, Jeff Brasen wrote:
> Add code to use a token attached to GICC to generate _CPC object on cpus.
>
>
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>
> ---
>
> .../SsdtCpuTopologyGenerator.c | 223 +++++++++++++++++-
>
> 1 file changed, 217 insertions(+), 6 deletions(-)
>
>
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> index 8561f48e1f..5d41d57064 100644
>
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> @@ -76,6 +76,16 @@ GET_OBJECT_LIST (
>
> CM_ARM_LPI_INFO
>
> );
>
>
>
> +/**
>
> + This macro expands to a function that retrieves the CPC
>
> + information from the Configuration Manager.
>
> +*/
>
> +GET_OBJECT_LIST (
>
> + EObjNameSpaceArm,
>
> + EArmObjCpcInfo,
>
> + CM_ARM_CPC_INFO
>
> + );
>
> +
>
> /** Initialize the TokenTable.
>
>
>
> One entry should be allocated for each CM_ARM_PROC_HIERARCHY_INFO
>
> @@ -229,6 +239,183 @@ WriteAslName (
>
> return EFI_SUCCESS;
>
> }
>
>
>
> +/** Utility function to check if generic address points to NULL
>
> +
>
> + @param [in] Address Pointer to the Generic address
>
> +
>
> + @retval TRUE Address is system memory with an Address of 0.
>
> + @retval FALSE Address does not point to NULL.
>
> +**/
>
> +STATIC
>
> +BOOLEAN
>
> +EFIAPI
>
> +IsNullGenericAddress (
[SAMI] Move this function to
DynamicTablesPkg\Library\Common\AmlLib\CodeGen\AmlCodeGen.c
>
> + IN EFI_ACPI_6_4_GENERIC_ADDRESS_STRUCTURE *Address
>
> + )
>
> +{
>
> + if ((Address == NULL) ||
>
> + ((Address->AddressSpaceId == EFI_ACPI_6_4_SYSTEM_MEMORY) &&
>
> + (Address->Address == 0x0)))
>
> + {
>
> + return TRUE;
>
> + }
>
> +
>
> + return FALSE;
>
> +}
>
> +
>
> +/** Create and add an _CPC Node to Cpu Node.
>
> +
>
> + For instance, transform an AML node from:
>
> + Device (C002)
>
> + {
>
> + Name (_UID, 2)
>
> + Name (_HID, "ACPI0007")
>
> + }
>
> +
>
> + To:
>
> + Device (C002)
>
> + {
>
> + Name (_UID, 2)
>
> + Name (_HID, "ACPI0007")
>
> + Name(_CPC, Package()
>
> + {
>
> + NumEntries, // Integer
>
> + Revision, // Integer
>
> + HighestPerformance, // Integer or Buffer (Resource Descriptor)
>
> + NominalPerformance, // Integer or Buffer (Resource Descriptor)
>
> + LowestNonlinearPerformance, // Integer or Buffer (Resource Descriptor)
>
> + LowestPerformance, // Integer or Buffer (Resource Descriptor)
>
> + GuaranteedPerformanceRegister, // Buffer (Resource Descriptor)
>
> + DesiredPerformanceRegister , // Buffer (Resource Descriptor)
>
> + MinimumPerformanceRegister , // Buffer (Resource Descriptor)
>
> + MaximumPerformanceRegister , // Buffer (Resource Descriptor)
>
> + PerformanceReductionToleranceRegister, // Buffer (Resource Descriptor)
>
> + TimeWindowRegister, // Buffer (Resource Descriptor)
>
> + CounterWraparoundTime, // Integer or Buffer (Resource Descriptor)
>
> + ReferencePerformanceCounterRegister, // Buffer (Resource Descriptor)
>
> + DeliveredPerformanceCounterRegister, // Buffer (Resource Descriptor)
>
> + PerformanceLimitedRegister, // Buffer (Resource Descriptor)
>
> + CPPCEnableRegister // Buffer (Resource Descriptor)
>
> + AutonomousSelectionEnable, // Integer or Buffer (Resource Descriptor)
>
> + AutonomousActivityWindowRegister, // Buffer (Resource Descriptor)
>
> + EnergyPerformancePreferenceRegister, // Buffer (Resource Descriptor)
>
> + ReferencePerformance // Integer or Buffer (Resource Descriptor)
>
> + LowestFrequency, // Integer or Buffer (Resource Descriptor)
>
> + NominalFrequency // Integer or Buffer (Resource Descriptor)
>
> + })
>
> + }
>
> +
>
> + @param [in] Generator The SSDT Cpu Topology generator.
>
> + @param [in] CfgMgrProtocol Pointer to the Configuration Manager
>
> + Protocol Interface.
>
> + @param [in] ProcHierarchyNodeInfo CM_ARM_PROC_HIERARCHY_INFO describing
>
> + the Cpu.
>
> + @param [in] Node CPU Node to which the _CPC node is
>
> + attached.
>
> +
>
> + @retval EFI_SUCCESS The function completed successfully.
>
> + @retval EFI_INVALID_PARAMETER Invalid parameter.
>
> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +CreateAmlCpcNode (
>
> + IN ACPI_CPU_TOPOLOGY_GENERATOR *Generator,
>
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
>
> + IN CM_ARM_GICC_INFO *GicCInfo,
>
> + IN AML_OBJECT_NODE_HANDLE *Node
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + CM_ARM_CPC_INFO *CpcInfo;
>
> +
>
> + Status = GetEArmObjCpcInfo (
>
> + CfgMgrProtocol,
>
> + GicCInfo->CpcToken,
>
> + &CpcInfo,
>
> + NULL
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT (0);
>
> + return Status;
>
> + }
>
> +
>
> + Status = AmlCreateCpcNode (
[SAMI] The number of parameters to this functions can be reduced to just
pass CpcInfo.
>
> + CpcInfo->Revision,
>
> + IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) ?
>
> + NULL :
>
> + &CpcInfo->HighestPerformanceBuffer,
>
> + CpcInfo->HighestPerformanceInteger,
>
> + IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) ?
>
> + NULL :
>
> + &CpcInfo->NominalPerformanceBuffer,
>
> + CpcInfo->NominalPerformanceInteger,
>
> + IsNullGenericAddress (&CpcInfo->LowestNonlinearPerformanceBuffer) ?
>
> + NULL :
>
> + &CpcInfo->LowestNonlinearPerformanceBuffer,
>
> + CpcInfo->LowestNonlinearPerformanceInteger,
>
> + IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) ?
>
> + NULL :
>
> + &CpcInfo->LowestPerformanceBuffer,
>
> + CpcInfo->LowestPerformanceInteger,
>
> + IsNullGenericAddress (&CpcInfo->GuaranteedPerformanceRegister) ?
>
> + NULL :
>
> + &CpcInfo->GuaranteedPerformanceRegister,
>
> + IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) ?
>
> + NULL :
>
> + &CpcInfo->DesiredPerformanceRegister,
>
> + IsNullGenericAddress (&CpcInfo->MinimumPerformanceRegister) ?
>
> + NULL :
>
> + &CpcInfo->MinimumPerformanceRegister,
>
> + IsNullGenericAddress (&CpcInfo->MaximumPerformanceRegister) ?
>
> + NULL :
>
> + &CpcInfo->MaximumPerformanceRegister,
>
> + IsNullGenericAddress (&CpcInfo->PerformanceReductionToleranceRegister) ?
>
> + NULL :
>
> + &CpcInfo->PerformanceReductionToleranceRegister,
>
> + IsNullGenericAddress (&CpcInfo->TimeWindowRegister) ?
>
> + NULL :
>
> + &CpcInfo->TimeWindowRegister,
>
> + IsNullGenericAddress (&CpcInfo->CounterWraparoundTimeBuffer) ?
>
> + NULL :
>
> + &CpcInfo->CounterWraparoundTimeBuffer,
>
> + CpcInfo->CounterWraparoundTimeInteger,
>
> + &CpcInfo->ReferencePerformanceCounterRegister,
>
> + &CpcInfo->DeliveredPerformanceCounterRegister,
>
> + &CpcInfo->PerformanceLimitedRegister,
>
> + IsNullGenericAddress (&CpcInfo->CPPCEnableRegister) ?
>
> + NULL :
>
> + &CpcInfo->CPPCEnableRegister,
>
> + IsNullGenericAddress (&CpcInfo->AutonomousSelectionEnableBuffer) ?
>
> + NULL :
>
> + &CpcInfo->AutonomousSelectionEnableBuffer,
>
> + CpcInfo->AutonomousSelectionEnableInteger,
>
> + IsNullGenericAddress (&CpcInfo->AutonomousActivityWindowRegister) ?
>
> + NULL :
>
> + &CpcInfo->AutonomousActivityWindowRegister,
>
> + IsNullGenericAddress (&CpcInfo->EnergyPerformancePreferenceRegister) ?
>
> + NULL :
>
> + &CpcInfo->EnergyPerformancePreferenceRegister,
>
> + IsNullGenericAddress (&CpcInfo->ReferencePerformanceBuffer) ?
>
> + NULL :
>
> + &CpcInfo->ReferencePerformanceBuffer,
>
> + CpcInfo->ReferencePerformanceInteger,
>
> + IsNullGenericAddress (&CpcInfo->LowestFrequencyBuffer) ?
>
> + NULL :
>
> + &CpcInfo->LowestFrequencyBuffer,
>
> + CpcInfo->LowestFrequencyInteger,
>
> + IsNullGenericAddress (&CpcInfo->NominalFrequencyBuffer) ?
>
> + NULL :
>
> + &CpcInfo->NominalFrequencyBuffer,
>
> + CpcInfo->NominalFrequencyInteger,
>
> + Node,
>
> + NULL
>
> + );
>
> + ASSERT_EFI_ERROR (Status);
>
> + return Status;
>
> +}
>
> +
>
> /** Create and add an _LPI method to Cpu/Cluster Node.
>
>
>
> For instance, transform an AML node from:
>
> @@ -581,7 +768,20 @@ CreateAmlCpuFromProcHierarchy (
>
> // CM_ARM_PROC_HIERARCHY_INFO, create an _LPI method returning them.
>
> if (ProcHierarchyNodeInfo->LpiToken != CM_NULL_TOKEN) {
>
> Status = CreateAmlLpiMethod (Generator, ProcHierarchyNodeInfo, CpuNode);
>
> - ASSERT_EFI_ERROR (Status);
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT_EFI_ERROR (Status);
>
> + return Status;
>
> + }
>
> + }
>
> +
>
> + // If a CPC info is associated with the
>
> + // GicCinfo, create an _CPC method returning them.
>
> + if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
>
> + Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT_EFI_ERROR (Status);
>
> + return Status;
>
> + }
>
> }
>
>
>
> return Status;
>
> @@ -934,10 +1134,11 @@ CreateTopologyFromGicC (
>
> IN AML_OBJECT_NODE_HANDLE ScopeNode
>
> )
>
> {
>
> - EFI_STATUS Status;
>
> - CM_ARM_GICC_INFO *GicCInfo;
>
> - UINT32 GicCInfoCount;
>
> - UINT32 Index;
>
> + EFI_STATUS Status;
>
> + CM_ARM_GICC_INFO *GicCInfo;
>
> + UINT32 GicCInfoCount;
>
> + UINT32 Index;
>
> + AML_OBJECT_NODE_HANDLE CpuNode;
>
>
>
> ASSERT (Generator != NULL);
>
> ASSERT (CfgMgrProtocol != NULL);
>
> @@ -961,12 +1162,22 @@ CreateTopologyFromGicC (
>
> ScopeNode,
>
> &GicCInfo[Index],
>
> Index,
>
> - NULL
>
> + &CpuNode
>
> );
>
> if (EFI_ERROR (Status)) {
>
> ASSERT (0);
>
> break;
>
> }
>
> +
>
> + // If a CPC info is associated with the
>
> + // GicCinfo, create an _CPC method returning them.
>
> + if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
>
> + Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, &GicCInfo[Index], CpuNode);
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT_EFI_ERROR (Status);
>
> + break;
>
> + }
>
> + }
>
> } // for
>
>
>
> return Status;
>
prev parent reply other threads:[~2022-09-15 15:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-15 13:10 [PATCH v3 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
2022-09-15 13:10 ` [PATCH v3 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
2022-09-15 15:14 ` Sami Mujawar
2022-09-15 13:10 ` [PATCH v3 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
2022-09-15 15:16 ` Sami Mujawar
2022-09-15 13:10 ` [PATCH v3 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
2022-09-15 15:16 ` Sami Mujawar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3b4de5f5-fece-cab1-e27b-d07888390a6c@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox