[edk2-devel] [edk2-platforms][PATCH V2 2/8] Platform/Sgi: ACPI PPTT table for SGI-575 platform
Sami Mujawar
sami.mujawar at arm.com
Mon May 10 08:43:32 UTC 2021
Hi Pranav,
Please see my response inline marked [SAMI].
Regards,
Sami Mujawar
On 28/04/2021 01:12 PM, Pranav Madhu wrote:
> From: Pranav Madhu <Pranav.Madhu at arm.com>
>
> The SGI-575 platform includes two clusters with four single-thread CPUs.
> Each of the CPUs include 64KB L1 Data cache, 64KB L1 Instruction cache
> and 512KB L2 cache. Each cluster includes a 2MB L3 cache. Add PPTT table
> for SGI-575 platform with this information.
>
> Signed-off-by: Pranav Madhu <pranav.madhu at arm.com>
> ---
> Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 3 +-
> Platform/ARM/SgiPkg/AcpiTables/Sgi575/Pptt.aslc | 161 ++++++++++++++++++++
> 2 files changed, 163 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> index 2121fd39f2f0..b1ee16e98ea3 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> @@ -1,7 +1,7 @@
> ## @file
> # ACPI table data and ASL sources required to boot the platform.
> #
> -# Copyright (c) 2018, ARM Ltd. All rights reserved.
> +# Copyright (c) 2018 - 2021, ARM Ltd. All rights reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -22,6 +22,7 @@
> Mcfg.aslc
> Sgi575/Dsdt.asl
> Sgi575/Madt.aslc
> + Sgi575/Pptt.aslc
> Spcr.aslc
> Ssdt.asl
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Pptt.aslc b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Pptt.aslc
> new file mode 100644
> index 000000000000..3388a012dd55
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Pptt.aslc
> @@ -0,0 +1,161 @@
> +/** @file
> +* Processor Properties Topology Table (PPTT) for SGI-575 platform
> +*
> +* This file describes the topological structure of the processor block on the
> +* SGI-575 platform in the form as defined by ACPI PPTT table. The SGI-575
> +* platform includes two clusters with four single-thread CPUS. Each of the CPUs
> +* include 64KB L1 Data cache, 64KB L1 Instruction cache and 512KB L2 cache.
> +* Each cluster includes a 2MB L3 cache.
> +*
> +* Copyright (c) 2021, ARM Limited. All rights reserved.
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +* @par Specification Reference:
> +* - ACPI 6.3, Chapter 5, Section 5.2.29, Processor Properties Topology Table
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/ArmLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "SgiPlatform.h"
> +#include "SgiAcpiHeader.h"
> +
> +#define PPTT_CORE_INIT(pid, cid, cpuid) \
[SAMI] Can you add a doxygen style header describing the parameters. As
it stands the meaning of the macro parameter is not intutive.
I can see the difficulty if PPTT_CORE_INIT() were to be a generic macro,
in which case it may be better to describe the parameters as affinity
levels.
However, in the current case this macro is local, so
- the macro could be modified to suit parameters specific to this file
OR
- Describe the macro parameters as affinity levels and add a Doxygen
function style comment header clarifying the parameter usage.
[/SAMI]
> + { \
> + /* Parameters for CPU Core */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT ( \
> + OFFSET_OF (RD_PPTT_CORE, DCache), /* Length */ \
> + PPTT_PROCESSOR_CORE_FLAGS, /* Flag */ \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid]), /* Parent */ \
> + ((pid << 3) | (cid << 2) | cpuid), /* ACPI Id */ \
> + 2 /* Num of private resource */ \
> + ), \
> + \
> + /* Offsets of the private resources */ \
> + { \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid].Core[cpuid].DCache), \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid].Core[cpuid].ICache) \
> + }, \
> + \
> + /* L1 data cache parameters */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT ( \
> + PPTT_CACHE_STRUCTURE_FLAGS, /* Flag */ \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid].Core[cpuid].L2Cache), \
> + /* Next level of cache */ \
> + SIZE_64KB, /* Size */ \
> + 64, /* Num of sets */ \
> + 16, /* Associativity */ \
> + PPTT_DATA_CACHE_ATTR, /* Attributes */ \
> + 64 /* Line size */ \
> + ), \
> + \
> + /* L1 instruction cache parameters */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT ( \
> + PPTT_CACHE_STRUCTURE_FLAGS, /* Flag */ \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid].Core[cpuid].L2Cache), \
> + /* Next level of cache */ \
> + SIZE_64KB, /* Size */ \
> + 256, /* Num of sets */ \
> + 4, /* Associativity */ \
> + PPTT_INST_CACHE_ATTR, /* Attributes */ \
> + 64 /* Line size */ \
> + ), \
> + \
> + /* L2 cache parameters */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT ( \
> + PPTT_CACHE_STRUCTURE_FLAGS, /* Flag */ \
> + 0, /* Next level of cache */ \
> + SIZE_512KB, /* Size */ \
> + 1024, /* Num of sets */ \
> + 8, /* Associativity */ \
> + PPTT_UNIFIED_CACHE_ATTR, /* Attributes */ \
> + 64 /* Line size */ \
> + ), \
> + }
> +
> +#define PPTT_CLUSTER_INIT(pid, cid) \
> + { \
> + /* Parameters for Cluster */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT ( \
> + OFFSET_OF (RD_PPTT_CLUSTER, L3Cache), \
> + /* Length */ \
> + PPTT_PROCESSOR_CLUSTER_FLAGS, /* Flag */ \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package), /* Parent */ \
> + 0, /* ACPI Id */ \
> + 1 /* Num of private resource */ \
> + ), \
> + \
> + /* Offsets of the private resources */ \
> + OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE, \
> + Package.Cluster[cid].L3Cache), \
> + \
> + /* L3 cache parameters */ \
> + EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT ( \
> + PPTT_CACHE_STRUCTURE_FLAGS, /* Flag */ \
> + 0, /* Next level of cache */ \
> + SIZE_2MB, /* Size */ \
> + 2048, /* Num of sets */ \
> + 16, /* Associativity */ \
> + PPTT_UNIFIED_CACHE_ATTR, /* Attributes */ \
> + 64 /* Line size */ \
> + ), \
> + \
> + /* Initialize child cores */ \
> + { \
> + PPTT_CORE_INIT (pid, cid, 0), \
> + PPTT_CORE_INIT (pid, cid, 1), \
> + PPTT_CORE_INIT (pid, cid, 2), \
> + PPTT_CORE_INIT (pid, cid, 3) \
> + } \
> + }
> +
> +#pragma pack(1)
> +typedef struct {
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Package;
> + RD_PPTT_CLUSTER Cluster[CLUSTER_COUNT];
> +} SGI575_PPTT_PACKAGE;
> +
> +/*
> + * Processor Properties Topology Table
> + */
> +typedef struct {
> + EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER Header;
> + SGI575_PPTT_PACKAGE Package;
> +} EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE;
> +#pragma pack ()
> +
> +STATIC EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE Pptt = {
> + {
> + ARM_ACPI_HEADER (
> + EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> + EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,
> + EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION
> + )
> + },
> +
> + {
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT (
> + OFFSET_OF (SGI575_PPTT_PACKAGE, Cluster[0]),
> + PPTT_PROCESSOR_PACKAGE_FLAGS, 0, 0, 0
> + ),
> + {
> + PPTT_CLUSTER_INIT (0, 0),
> + PPTT_CLUSTER_INIT (0, 1)
> + }
> + }
> +};
> +
> +/*
> + * Reference the table being generated to prevent the optimizer from removing
> + * the data structure from the executable
> + */
> +VOID* CONST ReferenceAcpiTable = &Pptt;
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74864): https://edk2.groups.io/g/devel/message/74864
Mute This Topic: https://groups.io/mt/82427821/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list