public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Pranav Madhu <pranav.madhu@arm.com>, devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Pierre Gondois <pierre.gondois@arm.com>
Subject: Re: [edk2-platforms][PATCH V3 08/14] Platform/Sgi: ACPI PPTT table for RD-E1-Edge platform
Date: Tue, 11 May 2021 12:05:33 +0100	[thread overview]
Message-ID: <30847e62-4d2c-78b2-2378-d8a02ceeb210@arm.com> (raw)
In-Reply-To: <20210510200615.26879-9-pranav.madhu@arm.com>

Hi Pranav,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 10/05/2021 09:06 PM, Pranav Madhu wrote:
> The RD-E1-Edge platform includes two clusters with eight multi-thread
> CPUs. Each of the CPUs include 32KB L1 Data cache, 32KB L1 Instruction
> cache and 256KB L2 cache. Each cluster includes a 2MB L3 cache. The
> platform also includes a system level cache of 8MB. Add PPTT table for
> RD-E1-Edge platform with this information.
>
> Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
> ---
>   Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf |   3 +-
>   Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Pptt.aslc     | 252 ++++++++++++++++++++
>   2 files changed, 254 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> index 2dd2275665a2..04ef2bfcaa26 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> @@ -1,7 +1,7 @@
>   ## @file
>   #  ACPI table data and ASL sources required to boot the platform.
>   #
> -#  Copyright (c) 2018-2020, ARM Ltd. All rights reserved.
> +#  Copyright (c) 2018-2021, ARM Ltd. All rights reserved.
>   #
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -23,6 +23,7 @@
>     Mcfg.aslc
>     RdE1Edge/Dsdt.asl
>     RdE1Edge/Madt.aslc
> +  RdE1Edge/Pptt.aslc
>     Spcr.aslc
>     Ssdt.asl
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Pptt.aslc b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Pptt.aslc
> new file mode 100644
> index 000000000000..91baab73d108
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Pptt.aslc
> @@ -0,0 +1,252 @@
> +/** @file
> +* Processor Properties Topology Table (PPTT) for RD-E1-Edge platform
> +*
> +* This file describes the topological structure of the processor block on the
> +* RD-E1-Edge platform in the form as defined by ACPI PPTT table. The RD-E1-Edge
> +* platform includes two clusters with eight dual-thread CPUS. Each of the CPUs
> +* include 32KB L1 Data cache, 32KB L1 Instruction cache and 256KB L2 cache.
> +* Each cluster includes a 2MB L3 cache. The platform also includes a system
> +* level cache of 8MB.
> +*
> +* 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 THREAD_PER_CORE_E1   2
> +
> +/*!
> +   \brief Define helper macro for populating processor thread information.
> +   \param PackageId Package instance number.
> +   \param ClusterId Cluster instance number.
> +   \param CpuId     CPU instance number.
> +   \param ThreadId  CPU thread number.
> +*/
> +#define PPTT_THREAD_INIT(PackageId, ClusterId, CpuId, ThreadId)                \
> +  {                                                                            \
> +    EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT (                               \
> +      sizeof (RDE1EDGE_PPTT_THREAD),        /* Length */                       \
> +      PPTT_PROCESSOR_THREAD_FLAGS,          /* Flag */                         \
> +      OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,             \
> +        Package.Cluster[ClusterId].Core[CpuId]),  /* Parent */                 \
> +      ((PackageId << 5) | (ClusterId << 4) | (CpuId << 1) | ThreadId),         \
> +                                            /* ACPI Id */                      \
> +      0                                     /* Num of private resource */      \
> +    )                                                                          \
> +  }
> +
> +/*!
> +   \brief Define helper macro for populating processor core information.
> +   \param PackageId Package instance number.
> +   \param ClusterId Cluster instance number.
> +   \param CpuId     CPU instance number.
> +*/
> +#define PPTT_CORE_INIT(PackageId, ClusterId, CpuId)                            \
> +  {                                                                            \
> +    /* Parameters for CPU Core */                                              \
> +    EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT (                               \
> +      OFFSET_OF (RDE1EDGE_PPTT_CORE, DCache),   /* Length */                   \
> +      PPTT_PROCESSOR_CORE_THREADED_FLAGS,       /* Flag */                     \
> +      OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,             \
> +        Package.Cluster[ClusterId]),            /* Parent */                   \
> +      0,                                        /* ACPI Id */                  \
> +      2                                         /* Num of private resource */  \
> +    ),                                                                         \
> +                                                                               \
> +    /* Offsets of the private resources */                                     \
> +    {                                                                          \
> +      OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,             \
> +        Package.Cluster[ClusterId].Core[CpuId].DCache),                        \
> +      OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,             \
> +        Package.Cluster[ClusterId].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[ClusterId].Core[CpuId].L2Cache),                       \
> +                                            /* Next level of cache */          \
> +      SIZE_32KB,                            /* Size */                         \
> +      128,                                  /* Num of sets */                  \
> +      4,                                    /* 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[ClusterId].Core[CpuId].L2Cache),                       \
> +                                            /* Next level of cache */          \
> +      SIZE_32KB,                            /* Size */                         \
> +      128,                                  /* 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_256KB,                           /* Size */                         \
> +      1024,                                 /* Num of sets */                  \
> +      4,                                    /* Associativity */                \
> +      PPTT_UNIFIED_CACHE_ATTR,              /* Attributes */                   \
> +      64                                    /* Line size */                    \
> +    ),                                                                         \
> +                                                                               \
> +    /* Thread Initialization */                                                \
> +    {                                                                          \
> +      PPTT_THREAD_INIT (PackageId, ClusterId, CpuId, 0),                       \
> +      PPTT_THREAD_INIT (PackageId, ClusterId, CpuId, 1)                        \
> +    }                                                                          \
> +  }
> +
> +/*!
> +   \brief Define helper macro for populating processor container information.
> +   \param PackageId Package instance number.
> +   \param ClusterId Cluster instance number.
> +*/
> +#define PPTT_CLUSTER_INIT(PackageId, ClusterId)                                \
> +  {                                                                            \
> +    /* Parameters for Cluster */                                               \
> +    EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT (                               \
> +      OFFSET_OF (RDE1EDGE_PPTT_CLUSTER, L3Cache),  /* Length */                \
> +      PPTT_PROCESSOR_CLUSTER_THREADED_FLAGS,       /* Flag */                  \
[SAMI] I see that PPTT_PROCESSOR_CLUSTER_THREADED_FLAGS sets the ACPI ID
flag to invalid. Is there a reason for doing this?
Also, it looks like the DSDT for RD-E1-Edge platform does not have the
clusters definitions. Am I missing something here?
Can you take a look, please?
[/SAMI]
> +      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[ClusterId].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 (PackageId, ClusterId, 0),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 1),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 2),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 3),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 4),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 5),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 6),                                \
> +      PPTT_CORE_INIT (PackageId, ClusterId, 7)                                 \
> +    }                                                                          \
> +  }
> +
> +/*!
> +   \brief Define helper macro for populating SoC package information.
> +   \param PackageId Package instance number.
> +*/
> +#define PPTT_PACKAGE_INIT(PackageId)                                           \
> +  {                                                                            \
> +    EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_INIT (                               \
> +      OFFSET_OF (RDE1EDGE_PPTT_PACKAGE, Slc),                                  \
> +      PPTT_PROCESSOR_PACKAGE_FLAGS,                                            \
> +      0,                                                                       \
> +      0,                                                                       \
> +      1                                                                        \
> +    ),                                                                         \
> +                                                                               \
> +    /* Offsets of the private resources */                                     \
> +    OFFSET_OF (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE,               \
> +               Package.Slc),                                                   \
> +                                                                               \
> +    /* SLC parameters */                                                       \
> +    EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE_INIT (                                   \
> +      PPTT_CACHE_STRUCTURE_FLAGS,         /* Flag */                           \
> +      0,                                  /* Next level of cache */            \
> +      SIZE_8MB,                           /* Size */                           \
> +      8192,                               /* Num of sets */                    \
> +      16,                                 /* Associativity */                  \
> +      PPTT_UNIFIED_CACHE_ATTR,            /* Attributes */                     \
> +      64                                  /* Line size */                      \
> +    ),                                                                         \
> +                                                                               \
> +    {                                                                          \
> +      PPTT_CLUSTER_INIT (PackageId, 0),                                        \
> +      PPTT_CLUSTER_INIT (PackageId, 1),                                        \
> +    }                                                                          \
> +  }
> +
> +#pragma pack(1)
> +typedef struct {
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Thread;
> +} RDE1EDGE_PPTT_THREAD;
> +
> +typedef struct {
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  Core;
> +  UINT32                                 Offset[2];
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE      DCache;
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE      ICache;
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE      L2Cache;
> +  RDE1EDGE_PPTT_THREAD                   Thread[THREAD_PER_CORE_E1];
> +} RDE1EDGE_PPTT_CORE;
> +
> +typedef struct {
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  Cluster;
> +  UINT32                                 Offset;
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE      L3Cache;
> +  RDE1EDGE_PPTT_CORE                     Core[CORE_COUNT / THREAD_PER_CORE_E1];
> +} RDE1EDGE_PPTT_CLUSTER;
> +
> +typedef struct {
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  Package;
> +  UINT32                                 Offset;
> +  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE      Slc;
> +  RDE1EDGE_PPTT_CLUSTER                  Cluster[CLUSTER_COUNT];
> +} RDE1EDGE_PPTT_PACKAGE;
> +
> +/*
> + * Processor Properties Topology Table
> + */
> +typedef struct {
> +  EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER  Header;
> +  RDE1EDGE_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
> +    )
> +  },
> +
> +  PPTT_PACKAGE_INIT (0)
> +};
> +
> +/*
> + * 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.

  reply	other threads:[~2021-05-11 11:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10 20:06 [edk2-platforms][PATCH V3 00/14] Platform/Sgi: Add PPTT table for Neoverse Reference Design platforms Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 01/14] Platform/Sgi: Helper macros for PPTT Table Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 02/14] Platform/Sgi: Add CPU container for SGI-575 Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 03/14] Platform/Sgi: ACPI PPTT table for SGI-575 platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 04/14] Platform/Sgi: Add CPU container for RD-N1-Edge Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 05/14] Platform/Sgi: ACPI PPTT table for RD-N1-Edge platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 06/14] Platform/Sgi: Add DSDT ACPI table for RD-N1-Edge dual-chip platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 07/14] Platform/Sgi: ACPI PPTT table for RD-N1-Edge dual-chip Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 08/14] Platform/Sgi: ACPI PPTT table for RD-E1-Edge platform Pranav Madhu
2021-05-11 11:05   ` Sami Mujawar [this message]
2021-05-11 11:25     ` Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 09/14] Platform/Sgi: Add CPU container for RD-V1 platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 10/14] Platform/Sgi: ACPI PPTT Table " Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 11/14] Platform/Sgi: Add CPU container for RD-V1 quad-chip platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 12/14] Platform/Sgi: ACPI PPTT Table " Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 13/14] Platform/Sgi: Add CPU container for RD-N2 platform Pranav Madhu
2021-05-10 20:06 ` [edk2-platforms][PATCH V3 14/14] Platform/Sgi: ACPI PPTT table " Pranav Madhu
2021-05-11 11:09 ` [edk2-platforms][PATCH V3 00/14] Platform/Sgi: Add PPTT table for Neoverse Reference Design platforms Sami Mujawar
2021-05-11 11:17   ` Pranav Madhu
2021-05-11 14:14 ` Sami Mujawar

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=30847e62-4d2c-78b2-2378-d8a02ceeb210@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