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.web12.6634.1618305501189007356 for ; Tue, 13 Apr 2021 02:18:21 -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 5BCAD106F; Tue, 13 Apr 2021 02:18:20 -0700 (PDT) Received: from [10.57.22.244] (unknown [10.57.22.244]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 28F533F73B; Tue, 13 Apr 2021 02:18:19 -0700 (PDT) From: "PierreGondois" Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 1/8] Platform/Sgi: Helper macros for PPTT Table To: "devel@edk2.groups.io" , Pranav Madhu Cc: Ard Biesheuvel , Leif Lindholm , Sami Mujawar References: <20210402091208.16752-1-pranav.madhu@arm.com> <20210402091208.16752-2-pranav.madhu@arm.com> Message-ID: <04572474-09c9-056f-397b-ef38ca06175c@arm.com> Date: Tue, 13 Apr 2021 10:18:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <20210402091208.16752-2-pranav.madhu@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Pranav, > diff --git a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h=20 > b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > index 8d715de173c9..7ceb090a78e9 100644 > --- a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > +++ b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h > @@ -1,6 +1,6 @@ > =A0/** @file > =A0* > -*=A0 Copyright (c) 2018-2020, ARM Limited. All rights reserved. > +*=A0 Copyright (c) 2018-2021, ARM Limited. All rights reserved. > =A0* > =A0*=A0 SPDX-License-Identifier: BSD-2-Clause-Patent > =A0* > @@ -20,6 +20,132 @@ > =A0#define EFI_ACPI_ARM_CREATOR_ID SIGNATURE_32('A','R','M',' ') > =A0#define EFI_ACPI_ARM_CREATOR_REVISION 0x00000099 > > +#define CORE_COUNT=A0=A0=A0=A0=A0 FixedPcdGet32 (PcdCoreCount) > +#define CLUSTER_COUNT=A0=A0 FixedPcdGet32 (PcdClusterCount) > + > +#pragma pack(1) > +// PPTT processor core structure > +typedef struct { > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR=A0 Core; > +=A0 UINT32=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 Offset[2]; I think there should be 3 entries (DCache, ICache, L2Cache). Updating=20 this will require updating the other PPTT tables written. Would it be also possible to rename the field 'PrivateResources' as in=20 the spec ? Another question: what does 'RD_' stands for ? > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE=A0=A0=A0=A0=A0 DCache; > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE=A0=A0=A0=A0=A0 ICache; > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE=A0=A0=A0=A0=A0 L2Cache; > +} RD_PPTT_CORE; > + > +// PPTT processor cluster structure > +typedef struct { > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR=A0 Cluster; > +=A0 UINT32=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 Offset; > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE=A0=A0=A0=A0=A0 L3Cache; > +=A0 RD_PPTT_CORE Core[CORE_COUNT]; > +} RD_PPTT_CLUSTER; > + > +// PPTT processor cluster structure without cache > +typedef struct { > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR=A0 Cluster; > +=A0 UINT32=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 Offset; I think there is no need for an offset here. Updating this will require=20 updating the other PPTT tables written. > +=A0 RD_PPTT_CORE Core[CORE_COUNT]; > +} RD_PPTT_MINIMAL_CLUSTER; > + > +// PPTT processor package structure > +typedef struct { > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR=A0 Package; > +=A0 UINT32=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 Offset; > +=A0 EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE=A0=A0=A0=A0=A0 Slc; > +=A0 RD_PPTT_MINIMAL_CLUSTER Cluster[CLUSTER_COUNT]; > +} RD_PPTT_SLC_PACKAGE; > +#pragma pack () > + > +// > +// PPTT processor structure flags for different SoC components as=20 > defined in > +// ACPI 6.3 specification > +// > + [...] > > +// EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR > +#define EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT(Length, Flag,=20 > Parent,=A0=A0=A0=A0=A0=A0 \ > +=A0 ACPIProcessorID, NumberOfPrivateResource) \ I think it should be possible to remove the 'Length' parameter and=20 compute it as: sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + NumberOfPrivateResource * sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) + NumberOfPrivateResource * sizeof (UINT32) > + { \ > +=A0=A0=A0 EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 /* Type 0=20 > */=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 Length,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 = /* Length=20 > */=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + }, \ > +=A0=A0=A0 Flag,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= /* Processor=20 > flags */=A0=A0=A0 \ > +=A0=A0=A0 Parent,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 = /* Ref to=20 > parent node */ \ > +=A0=A0=A0 ACPIProcessorID,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* UID, as per=20 > MADT */=A0=A0 \ > +=A0=A0=A0 NumberOfPrivateResource=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Resource=20 > count */=A0=A0=A0=A0 \ > +=A0 } > + > +// EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE > +#define EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT(Flag, NextLevelCache,=20 > Size,=A0=A0=A0=A0 \ > +=A0 NoOfSets, Associativity, Attributes,=20 > LineSize)=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0 \ > + { \ > +=A0=A0=A0 EFI_ACPI_6_3_PPTT_TYPE_CACHE,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Type 1=20 > */=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE),=A0=A0=A0=A0=A0=A0= /* Length=20 > */=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 \ > + { \ > + EFI_ACPI_RESERVED_BYTE, \ > + EFI_ACPI_RESERVED_BYTE, \ > + }, \ > +=A0=A0=A0 Flag,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= /* Cache flags=20 > */=A0=A0=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 NextLevelCache,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Ref to next=20 > level */=A0 \ > +=A0=A0=A0 Size,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= /* Size in=20 > bytes */=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 NoOfSets,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* = Num of sets=20 > */=A0=A0=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 Associativity,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Num of ways=20 > */=A0=A0=A0=A0=A0=A0=A0 \ > +=A0=A0=A0 Attributes,=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Cache=20 > attributes */=A0=A0 \ > +=A0=A0=A0 LineSize=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* = Line size in=20 > bytes */ \ > +=A0 } > + > =A0#endif /* __SGI_ACPI_HEADER__ */ > --=20 > 2.17.1 Regards, Pierre