public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
@ 2018-03-08 13:13 Ard Biesheuvel
  2018-03-08 17:24 ` Jeremy Linton
  2018-04-27 10:58 ` Leif Lindholm
  0 siblings, 2 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-08 13:13 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, graeme.gregory, jeremy.linton, Ard Biesheuvel

Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
builds. This information is used by the OS to tune the scheduler.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
This produces the following topology after applying Jeremy's patches:

$ lstopo-no-graphics 
Machine (31GB)
  Package L#0 + L3 L#0 (4096KB)
    L2 L#0 (256KB)
      L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
      L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
    L2 L#1 (256KB)
      L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
      L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
    L2 L#2 (256KB)
      L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
      L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
    L2 L#3 (256KB)
      L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
      L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
    L2 L#4 (256KB)
      L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
      L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
    L2 L#5 (256KB)
      L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
      L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
    L2 L#6 (256KB)
      L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
      L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
    L2 L#7 (256KB)
      L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
      L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
    L2 L#8 (256KB)
      L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
      L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
    L2 L#9 (256KB)
      L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
      L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
    L2 L#10 (256KB)
      L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
      L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
    L2 L#11 (256KB)
      L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
      L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
  HostBridge L#0
    PCIBridge
      PCIBridge
        PCI 1b21:0612
          Block(Disk) L#0 "sda"
  HostBridge L#3
    PCI 10de:128b
      GPU L#1 "renderD128"
      GPU L#2 "card0"
      GPU L#3 "controlD64"

 Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
 Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221 ++++++++++++++++++++
 2 files changed, 222 insertions(+)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
index bca8354d1184..afee50df5c63 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
@@ -31,6 +31,7 @@ [Sources]
   Iort.aslc
   Madt.aslc
   Mcfg.aslc
+  Pptt.aslc
   Spcr.aslc
 
 [Packages]
diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
new file mode 100644
index 000000000000..615f324dd37c
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
@@ -0,0 +1,221 @@
+/** @file
+
+  Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <IndustryStandard/Acpi.h>
+
+#include "AcpiTables.h"
+
+#define FIELD_OFFSET(type, name)            __builtin_offsetof(type, name)
+
+#pragma pack(1)
+typedef struct {
+  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Core;
+  UINT32                                                    Offset[2];
+  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         DCache;
+  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         ICache;
+} SYNQUACER_PPTT_CORE;
+
+typedef struct {
+  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Cluster;
+  UINT32                                                    Offset[1];
+  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L2Cache;
+  SYNQUACER_PPTT_CORE                                       Cores[2];
+} SYNQUACER_PPTT_CLUSTER;
+
+typedef struct {
+  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Package;
+  UINT32                                                    Offset[1];
+  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L3Cache;
+  SYNQUACER_PPTT_CLUSTER                                    Clusters[12];
+} SYNQUACER_PPTT_PACKAGE;
+
+typedef struct {
+  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER   Pptt;
+  SYNQUACER_PPTT_PACKAGE                                    Packages[1];
+} SYNQUACER_PPTT_TABLE;
+#pragma pack()
+
+#define PPTT_CORE(pid, cid, id) {                                             \
+  {                                                                           \
+    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
+    FIELD_OFFSET (SYNQUACER_PPTT_CORE, DCache),                               \
+    {},                                                                       \
+    {                                                                         \
+      0,                                        /* PhysicalPackage */         \
+      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_VALID,     /* AcpiProcessorIdValid */    \
+    },                                                                        \
+    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
+                  Packages[pid].Clusters[cid]), /* Parent */                  \
+    2 * (cid) + (id),                           /* AcpiProcessorId */         \
+    2,                                          /* NumberOfPrivateResources */\
+  }, {                                                                        \
+    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
+                  Packages[pid].Clusters[cid].Cores[id].DCache),              \
+    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
+                  Packages[pid].Clusters[cid].Cores[id].ICache),              \
+  }, {                                                                        \
+    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
+    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
+    {},                                                                       \
+    {                                                                         \
+      1,          /* SizePropertyValid */                                     \
+      1,          /* NumberOfSetsValid */                                     \
+      1,          /* AssociativityValid */                                    \
+      0,          /* AllocationTypeValid */                                   \
+      1,          /* CacheTypeValid */                                        \
+      1,          /* WritePolicyValid */                                      \
+      1,          /* LineSizeValid */                                         \
+    },                                                                        \
+    0,            /* NextLevelOfCache */                                      \
+    SIZE_32KB,    /* Size */                                                  \
+    128,          /* NumberOfSets */                                          \
+    4,            /* Associativity */                                         \
+    {                                                                         \
+      0,                                                /* AllocationType */  \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                          \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
+    },                                                                        \
+    64            /* LineSize */                                              \
+  }, {                                                                        \
+    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
+    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
+    {},                                                                       \
+    {                                                                         \
+      1,          /* SizePropertyValid */                                     \
+      1,          /* NumberOfSetsValid */                                     \
+      1,          /* AssociativityValid */                                    \
+      0,          /* AllocationTypeValid */                                   \
+      1,          /* CacheTypeValid */                                        \
+      1,          /* WritePolicyValid */                                      \
+      1,          /* LineSizeValid */                                         \
+    },                                                                        \
+    0,            /* NextLevelOfCache */                                      \
+    SIZE_32KB,    /* Size */                                                  \
+    256,          /* NumberOfSets */                                          \
+    2,            /* Associativity */                                         \
+    {                                                                         \
+      0,                                                /* AllocationType */  \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                   \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
+    },                                                                        \
+    64            /* LineSize */                                              \
+  }                                                                           \
+}
+
+#define PPTT_CLUSTER(pid, cid) {                                              \
+  {                                                                           \
+    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
+    FIELD_OFFSET (SYNQUACER_PPTT_CLUSTER, L2Cache),                           \
+    {},                                                                       \
+    {                                                                         \
+      0,                                      /* PhysicalPackage */           \
+      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */      \
+    },                                                                        \
+    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid]), /* Parent */          \
+    0,                                        /* AcpiProcessorId */           \
+    1,                                        /* NumberOfPrivateResources */  \
+  }, {                                                                        \
+    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid].Clusters[cid].L2Cache), \
+  }, {                                                                        \
+    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
+    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
+    {},                                                                       \
+    {                                                                         \
+      1,          /* SizePropertyValid */                                     \
+      1,          /* NumberOfSetsValid */                                     \
+      1,          /* AssociativityValid */                                    \
+      0,          /* AllocationTypeValid */                                   \
+      1,          /* CacheTypeValid */                                        \
+      1,          /* WritePolicyValid */                                      \
+      1,          /* LineSizeValid */                                         \
+    },                                                                        \
+    0,            /* NextLevelOfCache */                                      \
+    SIZE_256KB,   /* Size */                                                  \
+    256,          /* NumberOfSets */                                          \
+    16,           /* Associativity */                                         \
+    {                                                                         \
+      0,                                                /* AllocationType */  \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                       \
+      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
+    },                                                                        \
+    64            /* LineSize */                                              \
+  }, {                                                                        \
+    PPTT_CORE(pid, cid, 0),                                                   \
+    PPTT_CORE(pid, cid, 1),                                                   \
+  }                                                                           \
+}
+
+STATIC SYNQUACER_PPTT_TABLE mSynQuacerPpttTable = {
+  {
+    __ACPI_HEADER(EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
+                  SYNQUACER_PPTT_TABLE,
+                  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION),
+  },
+  {
+    {
+      {
+        EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,
+        FIELD_OFFSET (SYNQUACER_PPTT_PACKAGE, L3Cache),
+        {},
+        {
+          1,                                      /* PhysicalPackage */
+          EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */
+        },
+        0,                                        /* Parent */
+        0,                                        /* AcpiProcessorId */
+        1,                                        /* NumberOfPrivateResources */
+      }, {
+        FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[0].L3Cache),
+      }, {
+        EFI_ACPI_6_2_PPTT_TYPE_CACHE,
+        sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),
+        {},
+        {
+          1,                                      /* SizePropertyValid */
+          1,                                      /* NumberOfSetsValid */
+          1,                                      /* AssociativityValid */
+          0,                                      /* AllocationTypeValid */
+          1,                                      /* CacheTypeValid */
+          1,                                      /* WritePolicyValid */
+          1,                                      /* LineSizeValid */
+        },
+        0,                                        /* NextLevelOfCache */
+        SIZE_4MB,                                 /* Size */
+        4096,                                     /* NumberOfSets */
+        16,                                       /* Associativity */
+        {
+          0,                                      /* AllocationType */
+          EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,
+          EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,
+        },
+        64                                        /* LineSize */
+      }, {
+        PPTT_CLUSTER (0, 0),
+        PPTT_CLUSTER (0, 1),
+        PPTT_CLUSTER (0, 2),
+        PPTT_CLUSTER (0, 3),
+        PPTT_CLUSTER (0, 4),
+        PPTT_CLUSTER (0, 5),
+        PPTT_CLUSTER (0, 6),
+        PPTT_CLUSTER (0, 7),
+        PPTT_CLUSTER (0, 8),
+        PPTT_CLUSTER (0, 9),
+        PPTT_CLUSTER (0, 10),
+        PPTT_CLUSTER (0, 11),
+      }
+    }
+  }
+};
+
+VOID* CONST ReferenceAcpiTable = &mSynQuacerPpttTable;
-- 
2.15.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 13:13 [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology Ard Biesheuvel
@ 2018-03-08 17:24 ` Jeremy Linton
  2018-03-08 17:27   ` Ard Biesheuvel
  2018-04-27 10:58 ` Leif Lindholm
  1 sibling, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2018-03-08 17:24 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: leif.lindholm, graeme.gregory

Hi,

On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
> builds. This information is used by the OS to tune the scheduler.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> This produces the following topology after applying Jeremy's patches:
> 
> $ lstopo-no-graphics
> Machine (31GB)
>    Package L#0 + L3 L#0 (4096KB)
>      L2 L#0 (256KB)
>        L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>        L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>      L2 L#1 (256KB)
>        L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>        L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>      L2 L#2 (256KB)
>        L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>        L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>      L2 L#3 (256KB)
>        L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>        L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>      L2 L#4 (256KB)
>        L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>        L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>      L2 L#5 (256KB)
>        L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>        L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>      L2 L#6 (256KB)
>        L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>        L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>      L2 L#7 (256KB)
>        L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>        L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>      L2 L#8 (256KB)
>        L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>        L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>      L2 L#9 (256KB)
>        L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>        L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>      L2 L#10 (256KB)
>        L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>        L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>      L2 L#11 (256KB)
>        L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>        L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>    HostBridge L#0
>      PCIBridge
>        PCIBridge
>          PCI 1b21:0612
>            Block(Disk) L#0 "sda"
>    HostBridge L#3
>      PCI 10de:128b
>        GPU L#1 "renderD128"
>        GPU L#2 "card0"
>        GPU L#3 "controlD64"
> 
>   Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>   Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221 ++++++++++++++++++++

So, the above looks good. OTOH, this is yet another manually 
created/hard-coded ACPI table, subject to change every-time another SOC 
is released. I have a couple similar ones, but haven't post them because 
I believe the HiSi folks have done us a favor and created a table 
generator which does 90% of this work by probing the hardware, and 
creating a "compressed" representation of the table. Leaving the 
individual platforms to only fill out LLCs and such which can't be probed.

It would be great if the remaining HiSi bits were removed and the whole 
thing made generic enough to plug in to these individual platforms, so 
that they only need supply their nonstandard bits and the rest is taken 
care of.



>   2 files changed, 222 insertions(+)
> 
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> index bca8354d1184..afee50df5c63 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> @@ -31,6 +31,7 @@ [Sources]
>     Iort.aslc
>     Madt.aslc
>     Mcfg.aslc
> +  Pptt.aslc
>     Spcr.aslc
>   
>   [Packages]
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> new file mode 100644
> index 000000000000..615f324dd37c
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> @@ -0,0 +1,221 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +
> +#include "AcpiTables.h"
> +
> +#define FIELD_OFFSET(type, name)            __builtin_offsetof(type, name)
> +
> +#pragma pack(1)
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Core;
> +  UINT32                                                    Offset[2];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         DCache;
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         ICache;
> +} SYNQUACER_PPTT_CORE;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Cluster;
> +  UINT32                                                    Offset[1];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L2Cache;
> +  SYNQUACER_PPTT_CORE                                       Cores[2];
> +} SYNQUACER_PPTT_CLUSTER;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Package;
> +  UINT32                                                    Offset[1];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L3Cache;
> +  SYNQUACER_PPTT_CLUSTER                                    Clusters[12];
> +} SYNQUACER_PPTT_PACKAGE;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER   Pptt;
> +  SYNQUACER_PPTT_PACKAGE                                    Packages[1];
> +} SYNQUACER_PPTT_TABLE;
> +#pragma pack()
> +
> +#define PPTT_CORE(pid, cid, id) {                                             \
> +  {                                                                           \
> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
> +    FIELD_OFFSET (SYNQUACER_PPTT_CORE, DCache),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      0,                                        /* PhysicalPackage */         \
> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_VALID,     /* AcpiProcessorIdValid */    \
> +    },                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid]), /* Parent */                  \
> +    2 * (cid) + (id),                           /* AcpiProcessorId */         \
> +    2,                                          /* NumberOfPrivateResources */\
> +  }, {                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid].Cores[id].DCache),              \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid].Cores[id].ICache),              \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_32KB,    /* Size */                                                  \
> +    128,          /* NumberOfSets */                                          \
> +    4,            /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                          \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_32KB,    /* Size */                                                  \
> +    256,          /* NumberOfSets */                                          \
> +    2,            /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                   \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }                                                                           \
> +}
> +
> +#define PPTT_CLUSTER(pid, cid) {                                              \
> +  {                                                                           \
> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
> +    FIELD_OFFSET (SYNQUACER_PPTT_CLUSTER, L2Cache),                           \
> +    {},                                                                       \
> +    {                                                                         \
> +      0,                                      /* PhysicalPackage */           \
> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */      \
> +    },                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid]), /* Parent */          \
> +    0,                                        /* AcpiProcessorId */           \
> +    1,                                        /* NumberOfPrivateResources */  \
> +  }, {                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid].Clusters[cid].L2Cache), \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_256KB,   /* Size */                                                  \
> +    256,          /* NumberOfSets */                                          \
> +    16,           /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                       \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }, {                                                                        \
> +    PPTT_CORE(pid, cid, 0),                                                   \
> +    PPTT_CORE(pid, cid, 1),                                                   \
> +  }                                                                           \
> +}
> +
> +STATIC SYNQUACER_PPTT_TABLE mSynQuacerPpttTable = {
> +  {
> +    __ACPI_HEADER(EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> +                  SYNQUACER_PPTT_TABLE,
> +                  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION),
> +  },
> +  {
> +    {
> +      {
> +        EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,
> +        FIELD_OFFSET (SYNQUACER_PPTT_PACKAGE, L3Cache),
> +        {},
> +        {
> +          1,                                      /* PhysicalPackage */
> +          EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */
> +        },
> +        0,                                        /* Parent */
> +        0,                                        /* AcpiProcessorId */
> +        1,                                        /* NumberOfPrivateResources */
> +      }, {
> +        FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[0].L3Cache),
> +      }, {
> +        EFI_ACPI_6_2_PPTT_TYPE_CACHE,
> +        sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),
> +        {},
> +        {
> +          1,                                      /* SizePropertyValid */
> +          1,                                      /* NumberOfSetsValid */
> +          1,                                      /* AssociativityValid */
> +          0,                                      /* AllocationTypeValid */
> +          1,                                      /* CacheTypeValid */
> +          1,                                      /* WritePolicyValid */
> +          1,                                      /* LineSizeValid */
> +        },
> +        0,                                        /* NextLevelOfCache */
> +        SIZE_4MB,                                 /* Size */
> +        4096,                                     /* NumberOfSets */
> +        16,                                       /* Associativity */
> +        {
> +          0,                                      /* AllocationType */
> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,
> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,
> +        },
> +        64                                        /* LineSize */
> +      }, {
> +        PPTT_CLUSTER (0, 0),
> +        PPTT_CLUSTER (0, 1),
> +        PPTT_CLUSTER (0, 2),
> +        PPTT_CLUSTER (0, 3),
> +        PPTT_CLUSTER (0, 4),
> +        PPTT_CLUSTER (0, 5),
> +        PPTT_CLUSTER (0, 6),
> +        PPTT_CLUSTER (0, 7),
> +        PPTT_CLUSTER (0, 8),
> +        PPTT_CLUSTER (0, 9),
> +        PPTT_CLUSTER (0, 10),
> +        PPTT_CLUSTER (0, 11),
> +      }
> +    }
> +  }
> +};
> +
> +VOID* CONST ReferenceAcpiTable = &mSynQuacerPpttTable;
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 17:24 ` Jeremy Linton
@ 2018-03-08 17:27   ` Ard Biesheuvel
  2018-03-08 17:50     ` Jeremy Linton
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-08 17:27 UTC (permalink / raw)
  To: Jeremy Linton; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
> Hi,
>
>
> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>
>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>> builds. This information is used by the OS to tune the scheduler.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> This produces the following topology after applying Jeremy's patches:
>>
>> $ lstopo-no-graphics
>> Machine (31GB)
>>    Package L#0 + L3 L#0 (4096KB)
>>      L2 L#0 (256KB)
>>        L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>        L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>      L2 L#1 (256KB)
>>        L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>        L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>      L2 L#2 (256KB)
>>        L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>        L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>      L2 L#3 (256KB)
>>        L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>        L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>      L2 L#4 (256KB)
>>        L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>        L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>      L2 L#5 (256KB)
>>        L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>        L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>      L2 L#6 (256KB)
>>        L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>        L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>      L2 L#7 (256KB)
>>        L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>        L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>      L2 L#8 (256KB)
>>        L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>        L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>      L2 L#9 (256KB)
>>        L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>        L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>      L2 L#10 (256KB)
>>        L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>        L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>      L2 L#11 (256KB)
>>        L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>        L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>    HostBridge L#0
>>      PCIBridge
>>        PCIBridge
>>          PCI 1b21:0612
>>            Block(Disk) L#0 "sda"
>>    HostBridge L#3
>>      PCI 10de:128b
>>        GPU L#1 "renderD128"
>>        GPU L#2 "card0"
>>        GPU L#3 "controlD64"
>>
>>   Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>   Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>> ++++++++++++++++++++
>
>
> So, the above looks good. OTOH, this is yet another manually
> created/hard-coded ACPI table, subject to change every-time another SOC is
> released. I have a couple similar ones, but haven't post them because I
> believe the HiSi folks have done us a favor and created a table generator
> which does 90% of this work by probing the hardware, and creating a
> "compressed" representation of the table. Leaving the individual platforms
> to only fill out LLCs and such which can't be probed.
>
> It would be great if the remaining HiSi bits were removed and the whole
> thing made generic enough to plug in to these individual platforms, so that
> they only need supply their nonstandard bits and the rest is taken care of.
>

Yeah, I am aware of that. But to be honest, for a platform such as
this one, where the information is 100% static, I'd much rather have a
single .c file like this that never changes once you check it in.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 17:27   ` Ard Biesheuvel
@ 2018-03-08 17:50     ` Jeremy Linton
  2018-03-08 17:59       ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2018-03-08 17:50 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

Hi,

On 03/08/2018 11:27 AM, Ard Biesheuvel wrote:
> On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
>> Hi,
>>
>>
>> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>>
>>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>>> builds. This information is used by the OS to tune the scheduler.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> This produces the following topology after applying Jeremy's patches:
>>>
>>> $ lstopo-no-graphics
>>> Machine (31GB)
>>>     Package L#0 + L3 L#0 (4096KB)
>>>       L2 L#0 (256KB)
>>>         L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>>         L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>>       L2 L#1 (256KB)
>>>         L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>>         L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>>       L2 L#2 (256KB)
>>>         L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>>         L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>>       L2 L#3 (256KB)
>>>         L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>>         L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>>       L2 L#4 (256KB)
>>>         L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>>         L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>>       L2 L#5 (256KB)
>>>         L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>>         L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>>       L2 L#6 (256KB)
>>>         L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>>         L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>>       L2 L#7 (256KB)
>>>         L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>>         L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>>       L2 L#8 (256KB)
>>>         L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>>         L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>>       L2 L#9 (256KB)
>>>         L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>>         L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>>       L2 L#10 (256KB)
>>>         L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>>         L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>>       L2 L#11 (256KB)
>>>         L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>>         L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>>     HostBridge L#0
>>>       PCIBridge
>>>         PCIBridge
>>>           PCI 1b21:0612
>>>             Block(Disk) L#0 "sda"
>>>     HostBridge L#3
>>>       PCI 10de:128b
>>>         GPU L#1 "renderD128"
>>>         GPU L#2 "card0"
>>>         GPU L#3 "controlD64"
>>>
>>>    Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>>    Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>>> ++++++++++++++++++++
>>
>>
>> So, the above looks good. OTOH, this is yet another manually
>> created/hard-coded ACPI table, subject to change every-time another SOC is
>> released. I have a couple similar ones, but haven't post them because I
>> believe the HiSi folks have done us a favor and created a table generator
>> which does 90% of this work by probing the hardware, and creating a
>> "compressed" representation of the table. Leaving the individual platforms
>> to only fill out LLCs and such which can't be probed.
>>
>> It would be great if the remaining HiSi bits were removed and the whole
>> thing made generic enough to plug in to these individual platforms, so that
>> they only need supply their nonstandard bits and the rest is taken care of.
>>
> 
> Yeah, I am aware of that. But to be honest, for a platform such as
> this one, where the information is 100% static, I'd much rather have a
> single .c file like this that never changes once you check it in.
> 

Maybe, but even if there is never an identical machine with a few cores 
extra (or removed), you have to deal with the possibility that the 
standard is going to be updated (say to add a leaf node flag). If/when 
that happens you now have the technical debt of having to go manually 
touch the tables vs updating the generator code and being done with it 
across all the platforms. Particularly since other people are just going 
to take the same shortcut next time of just copy-pasting this table and 
tweaking it, rather than trying to create a library out of the HiSi code.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 17:50     ` Jeremy Linton
@ 2018-03-08 17:59       ` Ard Biesheuvel
  2018-03-08 20:38         ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-08 17:59 UTC (permalink / raw)
  To: Jeremy Linton; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

On 8 March 2018 at 17:50, Jeremy Linton <jeremy.linton@arm.com> wrote:
> Hi,
>
>
> On 03/08/2018 11:27 AM, Ard Biesheuvel wrote:
>>
>> On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>>>
>>>>
>>>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>>>> builds. This information is used by the OS to tune the scheduler.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>> This produces the following topology after applying Jeremy's patches:
>>>>
>>>> $ lstopo-no-graphics
>>>> Machine (31GB)
>>>>     Package L#0 + L3 L#0 (4096KB)
>>>>       L2 L#0 (256KB)
>>>>         L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>>>         L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>>>       L2 L#1 (256KB)
>>>>         L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>>>         L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>>>       L2 L#2 (256KB)
>>>>         L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>>>         L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>>>       L2 L#3 (256KB)
>>>>         L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>>>         L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>>>       L2 L#4 (256KB)
>>>>         L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>>>         L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>>>       L2 L#5 (256KB)
>>>>         L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>>>         L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>>>       L2 L#6 (256KB)
>>>>         L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>>>         L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>>>       L2 L#7 (256KB)
>>>>         L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>>>         L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>>>       L2 L#8 (256KB)
>>>>         L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>>>         L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>>>       L2 L#9 (256KB)
>>>>         L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>>>         L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>>>       L2 L#10 (256KB)
>>>>         L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>>>         L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>>>       L2 L#11 (256KB)
>>>>         L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>>>         L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>>>     HostBridge L#0
>>>>       PCIBridge
>>>>         PCIBridge
>>>>           PCI 1b21:0612
>>>>             Block(Disk) L#0 "sda"
>>>>     HostBridge L#3
>>>>       PCI 10de:128b
>>>>         GPU L#1 "renderD128"
>>>>         GPU L#2 "card0"
>>>>         GPU L#3 "controlD64"
>>>>
>>>>    Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>>>    Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>>>> ++++++++++++++++++++
>>>
>>>
>>>
>>> So, the above looks good. OTOH, this is yet another manually
>>> created/hard-coded ACPI table, subject to change every-time another SOC
>>> is
>>> released. I have a couple similar ones, but haven't post them because I
>>> believe the HiSi folks have done us a favor and created a table generator
>>> which does 90% of this work by probing the hardware, and creating a
>>> "compressed" representation of the table. Leaving the individual
>>> platforms
>>> to only fill out LLCs and such which can't be probed.
>>>
>>> It would be great if the remaining HiSi bits were removed and the whole
>>> thing made generic enough to plug in to these individual platforms, so
>>> that
>>> they only need supply their nonstandard bits and the rest is taken care
>>> of.
>>>
>>
>> Yeah, I am aware of that. But to be honest, for a platform such as
>> this one, where the information is 100% static, I'd much rather have a
>> single .c file like this that never changes once you check it in.
>>
>
> Maybe, but even if there is never an identical machine with a few cores
> extra (or removed), you have to deal with the possibility that the standard
> is going to be updated (say to add a leaf node flag). If/when that happens
> you now have the technical debt of having to go manually touch the tables vs
> updating the generator code and being done with it across all the platforms.
> Particularly since other people are just going to take the same shortcut
> next time of just copy-pasting this table and tweaking it, rather than
> trying to create a library out of the HiSi code.

I agree up to a point, and I think we are conflating two different
things here. I am all for abstracting PPTT table generation, but only
if it doesn't move processing of information known at compile time to
runtime.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 17:59       ` Ard Biesheuvel
@ 2018-03-08 20:38         ` Ard Biesheuvel
  2018-03-08 21:48           ` Jeremy Linton
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-08 20:38 UTC (permalink / raw)
  To: Jeremy Linton; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

On 8 March 2018 at 17:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 March 2018 at 17:50, Jeremy Linton <jeremy.linton@arm.com> wrote:
>> Hi,
>>
>>
>> On 03/08/2018 11:27 AM, Ard Biesheuvel wrote:
>>>
>>> On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>>>>
>>>>>
>>>>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>>>>> builds. This information is used by the OS to tune the scheduler.
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>> This produces the following topology after applying Jeremy's patches:
>>>>>
>>>>> $ lstopo-no-graphics
>>>>> Machine (31GB)
>>>>>     Package L#0 + L3 L#0 (4096KB)
>>>>>       L2 L#0 (256KB)
>>>>>         L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>>>>         L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>>>>       L2 L#1 (256KB)
>>>>>         L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>>>>         L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>>>>       L2 L#2 (256KB)
>>>>>         L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>>>>         L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>>>>       L2 L#3 (256KB)
>>>>>         L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>>>>         L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>>>>       L2 L#4 (256KB)
>>>>>         L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>>>>         L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>>>>       L2 L#5 (256KB)
>>>>>         L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>>>>         L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>>>>       L2 L#6 (256KB)
>>>>>         L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>>>>         L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>>>>       L2 L#7 (256KB)
>>>>>         L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>>>>         L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>>>>       L2 L#8 (256KB)
>>>>>         L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>>>>         L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>>>>       L2 L#9 (256KB)
>>>>>         L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>>>>         L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>>>>       L2 L#10 (256KB)
>>>>>         L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>>>>         L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>>>>       L2 L#11 (256KB)
>>>>>         L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>>>>         L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>>>>     HostBridge L#0
>>>>>       PCIBridge
>>>>>         PCIBridge
>>>>>           PCI 1b21:0612
>>>>>             Block(Disk) L#0 "sda"
>>>>>     HostBridge L#3
>>>>>       PCI 10de:128b
>>>>>         GPU L#1 "renderD128"
>>>>>         GPU L#2 "card0"
>>>>>         GPU L#3 "controlD64"
>>>>>
>>>>>    Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>>>>    Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>>>>> ++++++++++++++++++++
>>>>
>>>>
>>>>
>>>> So, the above looks good. OTOH, this is yet another manually
>>>> created/hard-coded ACPI table, subject to change every-time another SOC
>>>> is
>>>> released. I have a couple similar ones, but haven't post them because I
>>>> believe the HiSi folks have done us a favor and created a table generator
>>>> which does 90% of this work by probing the hardware, and creating a
>>>> "compressed" representation of the table. Leaving the individual
>>>> platforms
>>>> to only fill out LLCs and such which can't be probed.
>>>>
>>>> It would be great if the remaining HiSi bits were removed and the whole
>>>> thing made generic enough to plug in to these individual platforms, so
>>>> that
>>>> they only need supply their nonstandard bits and the rest is taken care
>>>> of.
>>>>
>>>
>>> Yeah, I am aware of that. But to be honest, for a platform such as
>>> this one, where the information is 100% static, I'd much rather have a
>>> single .c file like this that never changes once you check it in.
>>>
>>
>> Maybe, but even if there is never an identical machine with a few cores
>> extra (or removed), you have to deal with the possibility that the standard
>> is going to be updated (say to add a leaf node flag). If/when that happens
>> you now have the technical debt of having to go manually touch the tables vs
>> updating the generator code and being done with it across all the platforms.
>> Particularly since other people are just going to take the same shortcut
>> next time of just copy-pasting this table and tweaking it, rather than
>> trying to create a library out of the HiSi code.
>
> I agree up to a point, and I think we are conflating two different
> things here. I am all for abstracting PPTT table generation, but only
> if it doesn't move processing of information known at compile time to
> runtime.

BTW the architecture does not permit inferring cache geometry from the
CCSIDR field:

"""
The parameters NumSets, Associativity, and LineSize in these registers
define the architecturally visible parameters
that are required for the cache maintenance by Set/Way instructions.
They are not guaranteed to represent the actual
microarchitectural features of a design. You cannot make any inference
about the actual sizes of caches based on
these parameters.
"""

and we know that there are cores that describe their caches as having
a single set and a single way, because they don't implement
maintenance by set/way at all and can only be cleaned or invalidated
wholesale.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 20:38         ` Ard Biesheuvel
@ 2018-03-08 21:48           ` Jeremy Linton
  2018-03-08 22:56             ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2018-03-08 21:48 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

Hi,

On 03/08/2018 02:38 PM, Ard Biesheuvel wrote:
> On 8 March 2018 at 17:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 8 March 2018 at 17:50, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>> Hi,
>>>
>>>
>>> On 03/08/2018 11:27 AM, Ard Biesheuvel wrote:
>>>>
>>>> On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>>>>>
>>>>>>
>>>>>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>>>>>> builds. This information is used by the OS to tune the scheduler.
>>>>>>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>> ---
>>>>>> This produces the following topology after applying Jeremy's patches:
>>>>>>
>>>>>> $ lstopo-no-graphics
>>>>>> Machine (31GB)
>>>>>>      Package L#0 + L3 L#0 (4096KB)
>>>>>>        L2 L#0 (256KB)
>>>>>>          L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>>>>>          L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>>>>>        L2 L#1 (256KB)
>>>>>>          L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>>>>>          L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>>>>>        L2 L#2 (256KB)
>>>>>>          L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>>>>>          L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>>>>>        L2 L#3 (256KB)
>>>>>>          L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>>>>>          L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>>>>>        L2 L#4 (256KB)
>>>>>>          L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>>>>>          L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>>>>>        L2 L#5 (256KB)
>>>>>>          L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>>>>>          L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>>>>>        L2 L#6 (256KB)
>>>>>>          L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>>>>>          L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>>>>>        L2 L#7 (256KB)
>>>>>>          L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>>>>>          L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>>>>>        L2 L#8 (256KB)
>>>>>>          L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>>>>>          L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>>>>>        L2 L#9 (256KB)
>>>>>>          L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>>>>>          L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>>>>>        L2 L#10 (256KB)
>>>>>>          L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>>>>>          L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>>>>>        L2 L#11 (256KB)
>>>>>>          L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>>>>>          L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>>>>>      HostBridge L#0
>>>>>>        PCIBridge
>>>>>>          PCIBridge
>>>>>>            PCI 1b21:0612
>>>>>>              Block(Disk) L#0 "sda"
>>>>>>      HostBridge L#3
>>>>>>        PCI 10de:128b
>>>>>>          GPU L#1 "renderD128"
>>>>>>          GPU L#2 "card0"
>>>>>>          GPU L#3 "controlD64"
>>>>>>
>>>>>>     Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>>>>>     Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>>>>>> ++++++++++++++++++++
>>>>>
>>>>>
>>>>>
>>>>> So, the above looks good. OTOH, this is yet another manually
>>>>> created/hard-coded ACPI table, subject to change every-time another SOC
>>>>> is
>>>>> released. I have a couple similar ones, but haven't post them because I
>>>>> believe the HiSi folks have done us a favor and created a table generator
>>>>> which does 90% of this work by probing the hardware, and creating a
>>>>> "compressed" representation of the table. Leaving the individual
>>>>> platforms
>>>>> to only fill out LLCs and such which can't be probed.
>>>>>
>>>>> It would be great if the remaining HiSi bits were removed and the whole
>>>>> thing made generic enough to plug in to these individual platforms, so
>>>>> that
>>>>> they only need supply their nonstandard bits and the rest is taken care
>>>>> of.
>>>>>
>>>>
>>>> Yeah, I am aware of that. But to be honest, for a platform such as
>>>> this one, where the information is 100% static, I'd much rather have a
>>>> single .c file like this that never changes once you check it in.
>>>>
>>>
>>> Maybe, but even if there is never an identical machine with a few cores
>>> extra (or removed), you have to deal with the possibility that the standard
>>> is going to be updated (say to add a leaf node flag). If/when that happens
>>> you now have the technical debt of having to go manually touch the tables vs
>>> updating the generator code and being done with it across all the platforms.
>>> Particularly since other people are just going to take the same shortcut
>>> next time of just copy-pasting this table and tweaking it, rather than
>>> trying to create a library out of the HiSi code.
>>
>> I agree up to a point, and I think we are conflating two different
>> things here. I am all for abstracting PPTT table generation, but only
>> if it doesn't move processing of information known at compile time to
>> runtime.
> 
> BTW the architecture does not permit inferring cache geometry from the
> CCSIDR field:
> 
> """
> The parameters NumSets, Associativity, and LineSize in these registers
> define the architecturally visible parameters
> that are required for the cache maintenance by Set/Way instructions.
> They are not guaranteed to represent the actual
> microarchitectural features of a design. You cannot make any inference
> about the actual sizes of caches based on
> these parameters.
> """
> 
> and we know that there are cores that describe their caches as having
> a single set and a single way, because they don't implement
> maintenance by set/way at all and can only be cleaned or invalidated
> wholesale.
> 

That is true in the architectural sense, but as we all know for many of 
these machines it can be inferred, and since the firmware blobs would be 
per machine it would be a question of overriding/correcting whatever is 
wrong per machine rather than having the entire table be per machine.

Put another way, you can support a wide swath of machines with A5x & 
A7x's with a single piece of code which only hard-codes the L3 (if any) 
information.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 21:48           ` Jeremy Linton
@ 2018-03-08 22:56             ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-08 22:56 UTC (permalink / raw)
  To: Jeremy Linton; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Graeme Gregory

On 8 March 2018 at 21:48, Jeremy Linton <jeremy.linton@arm.com> wrote:
> Hi,
>
>
> On 03/08/2018 02:38 PM, Ard Biesheuvel wrote:
>>
>> On 8 March 2018 at 17:59, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>>>
>>> On 8 March 2018 at 17:50, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 03/08/2018 11:27 AM, Ard Biesheuvel wrote:
>>>>>
>>>>>
>>>>> On 8 March 2018 at 17:24, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> On 03/08/2018 07:13 AM, Ard Biesheuvel wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Add a ACPI Processor Properties Topology Table (PPTT) to the
>>>>>>> SynQuacer
>>>>>>> builds. This information is used by the OS to tune the scheduler.
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>> ---
>>>>>>> This produces the following topology after applying Jeremy's patches:
>>>>>>>
>>>>>>> $ lstopo-no-graphics
>>>>>>> Machine (31GB)
>>>>>>>      Package L#0 + L3 L#0 (4096KB)
>>>>>>>        L2 L#0 (256KB)
>>>>>>>          L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>>>>>>          L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>>>>>>        L2 L#1 (256KB)
>>>>>>>          L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>>>>>>          L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>>>>>>        L2 L#2 (256KB)
>>>>>>>          L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>>>>>>          L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>>>>>>        L2 L#3 (256KB)
>>>>>>>          L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>>>>>>          L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>>>>>>        L2 L#4 (256KB)
>>>>>>>          L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>>>>>>          L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>>>>>>        L2 L#5 (256KB)
>>>>>>>          L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10
>>>>>>> (P#10)
>>>>>>>          L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11
>>>>>>> (P#11)
>>>>>>>        L2 L#6 (256KB)
>>>>>>>          L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12
>>>>>>> (P#12)
>>>>>>>          L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13
>>>>>>> (P#13)
>>>>>>>        L2 L#7 (256KB)
>>>>>>>          L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14
>>>>>>> (P#14)
>>>>>>>          L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15
>>>>>>> (P#15)
>>>>>>>        L2 L#8 (256KB)
>>>>>>>          L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16
>>>>>>> (P#16)
>>>>>>>          L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17
>>>>>>> (P#17)
>>>>>>>        L2 L#9 (256KB)
>>>>>>>          L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18
>>>>>>> (P#18)
>>>>>>>          L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19
>>>>>>> (P#19)
>>>>>>>        L2 L#10 (256KB)
>>>>>>>          L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20
>>>>>>> (P#20)
>>>>>>>          L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21
>>>>>>> (P#21)
>>>>>>>        L2 L#11 (256KB)
>>>>>>>          L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22
>>>>>>> (P#22)
>>>>>>>          L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23
>>>>>>> (P#23)
>>>>>>>      HostBridge L#0
>>>>>>>        PCIBridge
>>>>>>>          PCIBridge
>>>>>>>            PCI 1b21:0612
>>>>>>>              Block(Disk) L#0 "sda"
>>>>>>>      HostBridge L#3
>>>>>>>        PCI 10de:128b
>>>>>>>          GPU L#1 "renderD128"
>>>>>>>          GPU L#2 "card0"
>>>>>>>          GPU L#3 "controlD64"
>>>>>>>
>>>>>>>     Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>>>>>>     Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221
>>>>>>> ++++++++++++++++++++
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> So, the above looks good. OTOH, this is yet another manually
>>>>>> created/hard-coded ACPI table, subject to change every-time another
>>>>>> SOC
>>>>>> is
>>>>>> released. I have a couple similar ones, but haven't post them because
>>>>>> I
>>>>>> believe the HiSi folks have done us a favor and created a table
>>>>>> generator
>>>>>> which does 90% of this work by probing the hardware, and creating a
>>>>>> "compressed" representation of the table. Leaving the individual
>>>>>> platforms
>>>>>> to only fill out LLCs and such which can't be probed.
>>>>>>
>>>>>> It would be great if the remaining HiSi bits were removed and the
>>>>>> whole
>>>>>> thing made generic enough to plug in to these individual platforms, so
>>>>>> that
>>>>>> they only need supply their nonstandard bits and the rest is taken
>>>>>> care
>>>>>> of.
>>>>>>
>>>>>
>>>>> Yeah, I am aware of that. But to be honest, for a platform such as
>>>>> this one, where the information is 100% static, I'd much rather have a
>>>>> single .c file like this that never changes once you check it in.
>>>>>
>>>>
>>>> Maybe, but even if there is never an identical machine with a few cores
>>>> extra (or removed), you have to deal with the possibility that the
>>>> standard
>>>> is going to be updated (say to add a leaf node flag). If/when that
>>>> happens
>>>> you now have the technical debt of having to go manually touch the
>>>> tables vs
>>>> updating the generator code and being done with it across all the
>>>> platforms.
>>>> Particularly since other people are just going to take the same shortcut
>>>> next time of just copy-pasting this table and tweaking it, rather than
>>>> trying to create a library out of the HiSi code.
>>>
>>>
>>> I agree up to a point, and I think we are conflating two different
>>> things here. I am all for abstracting PPTT table generation, but only
>>> if it doesn't move processing of information known at compile time to
>>> runtime.
>>
>>
>> BTW the architecture does not permit inferring cache geometry from the
>> CCSIDR field:
>>
>> """
>> The parameters NumSets, Associativity, and LineSize in these registers
>> define the architecturally visible parameters
>> that are required for the cache maintenance by Set/Way instructions.
>> They are not guaranteed to represent the actual
>> microarchitectural features of a design. You cannot make any inference
>> about the actual sizes of caches based on
>> these parameters.
>> """
>>
>> and we know that there are cores that describe their caches as having
>> a single set and a single way, because they don't implement
>> maintenance by set/way at all and can only be cleaned or invalidated
>> wholesale.
>>
>
> That is true in the architectural sense, but as we all know for many of
> these machines it can be inferred, and since the firmware blobs would be per
> machine it would be a question of overriding/correcting whatever is wrong
> per machine rather than having the entire table be per machine.
>

Yes, but my point is that if you're doing that, why do you read the
cache registers at all, and not simply fill out all the info.

Firmware is tightly coupled to the platform anyway, unlike the kernel,
so I don't see the value in having runtime code that infers
information that never changes.

> Put another way, you can support a wide swath of machines with A5x & A7x's
> with a single piece of code which only hard-codes the L3 (if any)
> information.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-03-08 13:13 [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology Ard Biesheuvel
  2018-03-08 17:24 ` Jeremy Linton
@ 2018-04-27 10:58 ` Leif Lindholm
  2018-04-27 12:12   ` Ard Biesheuvel
  1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-04-27 10:58 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, graeme.gregory, jeremy.linton

On Thu, Mar 08, 2018 at 01:13:53PM +0000, Ard Biesheuvel wrote:
> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
> builds. This information is used by the OS to tune the scheduler.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Not my area of expertise, but looks reasonable.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
> This produces the following topology after applying Jeremy's patches:
> 
> $ lstopo-no-graphics 
> Machine (31GB)
>   Package L#0 + L3 L#0 (4096KB)
>     L2 L#0 (256KB)
>       L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>       L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>     L2 L#1 (256KB)
>       L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>       L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>     L2 L#2 (256KB)
>       L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>       L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>     L2 L#3 (256KB)
>       L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>       L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>     L2 L#4 (256KB)
>       L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>       L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>     L2 L#5 (256KB)
>       L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>       L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>     L2 L#6 (256KB)
>       L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>       L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>     L2 L#7 (256KB)
>       L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>       L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>     L2 L#8 (256KB)
>       L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>       L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>     L2 L#9 (256KB)
>       L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>       L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>     L2 L#10 (256KB)
>       L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>       L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>     L2 L#11 (256KB)
>       L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>       L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>   HostBridge L#0
>     PCIBridge
>       PCIBridge
>         PCI 1b21:0612
>           Block(Disk) L#0 "sda"
>   HostBridge L#3
>     PCI 10de:128b
>       GPU L#1 "renderD128"
>       GPU L#2 "card0"
>       GPU L#3 "controlD64"
> 
>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>  Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221 ++++++++++++++++++++
>  2 files changed, 222 insertions(+)
> 
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> index bca8354d1184..afee50df5c63 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
> @@ -31,6 +31,7 @@ [Sources]
>    Iort.aslc
>    Madt.aslc
>    Mcfg.aslc
> +  Pptt.aslc
>    Spcr.aslc
>  
>  [Packages]
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> new file mode 100644
> index 000000000000..615f324dd37c
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> @@ -0,0 +1,221 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +
> +#include "AcpiTables.h"
> +
> +#define FIELD_OFFSET(type, name)            __builtin_offsetof(type, name)
> +
> +#pragma pack(1)
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Core;
> +  UINT32                                                    Offset[2];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         DCache;
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         ICache;
> +} SYNQUACER_PPTT_CORE;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Cluster;
> +  UINT32                                                    Offset[1];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L2Cache;
> +  SYNQUACER_PPTT_CORE                                       Cores[2];
> +} SYNQUACER_PPTT_CLUSTER;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Package;
> +  UINT32                                                    Offset[1];
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L3Cache;
> +  SYNQUACER_PPTT_CLUSTER                                    Clusters[12];
> +} SYNQUACER_PPTT_PACKAGE;
> +
> +typedef struct {
> +  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER   Pptt;
> +  SYNQUACER_PPTT_PACKAGE                                    Packages[1];
> +} SYNQUACER_PPTT_TABLE;
> +#pragma pack()
> +
> +#define PPTT_CORE(pid, cid, id) {                                             \
> +  {                                                                           \
> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
> +    FIELD_OFFSET (SYNQUACER_PPTT_CORE, DCache),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      0,                                        /* PhysicalPackage */         \
> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_VALID,     /* AcpiProcessorIdValid */    \
> +    },                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid]), /* Parent */                  \
> +    2 * (cid) + (id),                           /* AcpiProcessorId */         \
> +    2,                                          /* NumberOfPrivateResources */\
> +  }, {                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid].Cores[id].DCache),              \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
> +                  Packages[pid].Clusters[cid].Cores[id].ICache),              \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_32KB,    /* Size */                                                  \
> +    128,          /* NumberOfSets */                                          \
> +    4,            /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                          \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_32KB,    /* Size */                                                  \
> +    256,          /* NumberOfSets */                                          \
> +    2,            /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                   \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }                                                                           \
> +}
> +
> +#define PPTT_CLUSTER(pid, cid) {                                              \
> +  {                                                                           \
> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
> +    FIELD_OFFSET (SYNQUACER_PPTT_CLUSTER, L2Cache),                           \
> +    {},                                                                       \
> +    {                                                                         \
> +      0,                                      /* PhysicalPackage */           \
> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */      \
> +    },                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid]), /* Parent */          \
> +    0,                                        /* AcpiProcessorId */           \
> +    1,                                        /* NumberOfPrivateResources */  \
> +  }, {                                                                        \
> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid].Clusters[cid].L2Cache), \
> +  }, {                                                                        \
> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
> +    {},                                                                       \
> +    {                                                                         \
> +      1,          /* SizePropertyValid */                                     \
> +      1,          /* NumberOfSetsValid */                                     \
> +      1,          /* AssociativityValid */                                    \
> +      0,          /* AllocationTypeValid */                                   \
> +      1,          /* CacheTypeValid */                                        \
> +      1,          /* WritePolicyValid */                                      \
> +      1,          /* LineSizeValid */                                         \
> +    },                                                                        \
> +    0,            /* NextLevelOfCache */                                      \
> +    SIZE_256KB,   /* Size */                                                  \
> +    256,          /* NumberOfSets */                                          \
> +    16,           /* Associativity */                                         \
> +    {                                                                         \
> +      0,                                                /* AllocationType */  \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                       \
> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
> +    },                                                                        \
> +    64            /* LineSize */                                              \
> +  }, {                                                                        \
> +    PPTT_CORE(pid, cid, 0),                                                   \
> +    PPTT_CORE(pid, cid, 1),                                                   \
> +  }                                                                           \
> +}
> +
> +STATIC SYNQUACER_PPTT_TABLE mSynQuacerPpttTable = {
> +  {
> +    __ACPI_HEADER(EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> +                  SYNQUACER_PPTT_TABLE,
> +                  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION),
> +  },
> +  {
> +    {
> +      {
> +        EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,
> +        FIELD_OFFSET (SYNQUACER_PPTT_PACKAGE, L3Cache),
> +        {},
> +        {
> +          1,                                      /* PhysicalPackage */
> +          EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */
> +        },
> +        0,                                        /* Parent */
> +        0,                                        /* AcpiProcessorId */
> +        1,                                        /* NumberOfPrivateResources */
> +      }, {
> +        FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[0].L3Cache),
> +      }, {
> +        EFI_ACPI_6_2_PPTT_TYPE_CACHE,
> +        sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),
> +        {},
> +        {
> +          1,                                      /* SizePropertyValid */
> +          1,                                      /* NumberOfSetsValid */
> +          1,                                      /* AssociativityValid */
> +          0,                                      /* AllocationTypeValid */
> +          1,                                      /* CacheTypeValid */
> +          1,                                      /* WritePolicyValid */
> +          1,                                      /* LineSizeValid */
> +        },
> +        0,                                        /* NextLevelOfCache */
> +        SIZE_4MB,                                 /* Size */
> +        4096,                                     /* NumberOfSets */
> +        16,                                       /* Associativity */
> +        {
> +          0,                                      /* AllocationType */
> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,
> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,
> +        },
> +        64                                        /* LineSize */
> +      }, {
> +        PPTT_CLUSTER (0, 0),
> +        PPTT_CLUSTER (0, 1),
> +        PPTT_CLUSTER (0, 2),
> +        PPTT_CLUSTER (0, 3),
> +        PPTT_CLUSTER (0, 4),
> +        PPTT_CLUSTER (0, 5),
> +        PPTT_CLUSTER (0, 6),
> +        PPTT_CLUSTER (0, 7),
> +        PPTT_CLUSTER (0, 8),
> +        PPTT_CLUSTER (0, 9),
> +        PPTT_CLUSTER (0, 10),
> +        PPTT_CLUSTER (0, 11),
> +      }
> +    }
> +  }
> +};
> +
> +VOID* CONST ReferenceAcpiTable = &mSynQuacerPpttTable;
> -- 
> 2.15.1
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology
  2018-04-27 10:58 ` Leif Lindholm
@ 2018-04-27 12:12   ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-04-27 12:12 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Graeme Gregory, Jeremy Linton

On 27 April 2018 at 12:58, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Mar 08, 2018 at 01:13:53PM +0000, Ard Biesheuvel wrote:
>> Add a ACPI Processor Properties Topology Table (PPTT) to the SynQuacer
>> builds. This information is used by the OS to tune the scheduler.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Not my area of expertise, but looks reasonable.
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Pushed as 0f980aeb3e4456668aa2c4092926a5303e242c0b


>> ---
>> This produces the following topology after applying Jeremy's patches:
>>
>> $ lstopo-no-graphics
>> Machine (31GB)
>>   Package L#0 + L3 L#0 (4096KB)
>>     L2 L#0 (256KB)
>>       L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
>>       L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)
>>     L2 L#1 (256KB)
>>       L1d L#2 (32KB) + L1i L#2 (32KB) + Core L#2 + PU L#2 (P#2)
>>       L1d L#3 (32KB) + L1i L#3 (32KB) + Core L#3 + PU L#3 (P#3)
>>     L2 L#2 (256KB)
>>       L1d L#4 (32KB) + L1i L#4 (32KB) + Core L#4 + PU L#4 (P#4)
>>       L1d L#5 (32KB) + L1i L#5 (32KB) + Core L#5 + PU L#5 (P#5)
>>     L2 L#3 (256KB)
>>       L1d L#6 (32KB) + L1i L#6 (32KB) + Core L#6 + PU L#6 (P#6)
>>       L1d L#7 (32KB) + L1i L#7 (32KB) + Core L#7 + PU L#7 (P#7)
>>     L2 L#4 (256KB)
>>       L1d L#8 (32KB) + L1i L#8 (32KB) + Core L#8 + PU L#8 (P#8)
>>       L1d L#9 (32KB) + L1i L#9 (32KB) + Core L#9 + PU L#9 (P#9)
>>     L2 L#5 (256KB)
>>       L1d L#10 (32KB) + L1i L#10 (32KB) + Core L#10 + PU L#10 (P#10)
>>       L1d L#11 (32KB) + L1i L#11 (32KB) + Core L#11 + PU L#11 (P#11)
>>     L2 L#6 (256KB)
>>       L1d L#12 (32KB) + L1i L#12 (32KB) + Core L#12 + PU L#12 (P#12)
>>       L1d L#13 (32KB) + L1i L#13 (32KB) + Core L#13 + PU L#13 (P#13)
>>     L2 L#7 (256KB)
>>       L1d L#14 (32KB) + L1i L#14 (32KB) + Core L#14 + PU L#14 (P#14)
>>       L1d L#15 (32KB) + L1i L#15 (32KB) + Core L#15 + PU L#15 (P#15)
>>     L2 L#8 (256KB)
>>       L1d L#16 (32KB) + L1i L#16 (32KB) + Core L#16 + PU L#16 (P#16)
>>       L1d L#17 (32KB) + L1i L#17 (32KB) + Core L#17 + PU L#17 (P#17)
>>     L2 L#9 (256KB)
>>       L1d L#18 (32KB) + L1i L#18 (32KB) + Core L#18 + PU L#18 (P#18)
>>       L1d L#19 (32KB) + L1i L#19 (32KB) + Core L#19 + PU L#19 (P#19)
>>     L2 L#10 (256KB)
>>       L1d L#20 (32KB) + L1i L#20 (32KB) + Core L#20 + PU L#20 (P#20)
>>       L1d L#21 (32KB) + L1i L#21 (32KB) + Core L#21 + PU L#21 (P#21)
>>     L2 L#11 (256KB)
>>       L1d L#22 (32KB) + L1i L#22 (32KB) + Core L#22 + PU L#22 (P#22)
>>       L1d L#23 (32KB) + L1i L#23 (32KB) + Core L#23 + PU L#23 (P#23)
>>   HostBridge L#0
>>     PCIBridge
>>       PCIBridge
>>         PCI 1b21:0612
>>           Block(Disk) L#0 "sda"
>>   HostBridge L#3
>>     PCI 10de:128b
>>       GPU L#1 "renderD128"
>>       GPU L#2 "card0"
>>       GPU L#3 "controlD64"
>>
>>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |   1 +
>>  Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc      | 221 ++++++++++++++++++++
>>  2 files changed, 222 insertions(+)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
>> index bca8354d1184..afee50df5c63 100644
>> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
>> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
>> @@ -31,6 +31,7 @@ [Sources]
>>    Iort.aslc
>>    Madt.aslc
>>    Mcfg.aslc
>> +  Pptt.aslc
>>    Spcr.aslc
>>
>>  [Packages]
>> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
>> new file mode 100644
>> index 000000000000..615f324dd37c
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
>> @@ -0,0 +1,221 @@
>> +/** @file
>> +
>> +  Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <IndustryStandard/Acpi.h>
>> +
>> +#include "AcpiTables.h"
>> +
>> +#define FIELD_OFFSET(type, name)            __builtin_offsetof(type, name)
>> +
>> +#pragma pack(1)
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Core;
>> +  UINT32                                                    Offset[2];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         DCache;
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         ICache;
>> +} SYNQUACER_PPTT_CORE;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Cluster;
>> +  UINT32                                                    Offset[1];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L2Cache;
>> +  SYNQUACER_PPTT_CORE                                       Cores[2];
>> +} SYNQUACER_PPTT_CLUSTER;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR                     Package;
>> +  UINT32                                                    Offset[1];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE                         L3Cache;
>> +  SYNQUACER_PPTT_CLUSTER                                    Clusters[12];
>> +} SYNQUACER_PPTT_PACKAGE;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER   Pptt;
>> +  SYNQUACER_PPTT_PACKAGE                                    Packages[1];
>> +} SYNQUACER_PPTT_TABLE;
>> +#pragma pack()
>> +
>> +#define PPTT_CORE(pid, cid, id) {                                             \
>> +  {                                                                           \
>> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_CORE, DCache),                               \
>> +    {},                                                                       \
>> +    {                                                                         \
>> +      0,                                        /* PhysicalPackage */         \
>> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_VALID,     /* AcpiProcessorIdValid */    \
>> +    },                                                                        \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
>> +                  Packages[pid].Clusters[cid]), /* Parent */                  \
>> +    2 * (cid) + (id),                           /* AcpiProcessorId */         \
>> +    2,                                          /* NumberOfPrivateResources */\
>> +  }, {                                                                        \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
>> +                  Packages[pid].Clusters[cid].Cores[id].DCache),              \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE,                                       \
>> +                  Packages[pid].Clusters[cid].Cores[id].ICache),              \
>> +  }, {                                                                        \
>> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
>> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
>> +    {},                                                                       \
>> +    {                                                                         \
>> +      1,          /* SizePropertyValid */                                     \
>> +      1,          /* NumberOfSetsValid */                                     \
>> +      1,          /* AssociativityValid */                                    \
>> +      0,          /* AllocationTypeValid */                                   \
>> +      1,          /* CacheTypeValid */                                        \
>> +      1,          /* WritePolicyValid */                                      \
>> +      1,          /* LineSizeValid */                                         \
>> +    },                                                                        \
>> +    0,            /* NextLevelOfCache */                                      \
>> +    SIZE_32KB,    /* Size */                                                  \
>> +    128,          /* NumberOfSets */                                          \
>> +    4,            /* Associativity */                                         \
>> +    {                                                                         \
>> +      0,                                                /* AllocationType */  \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                          \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
>> +    },                                                                        \
>> +    64            /* LineSize */                                              \
>> +  }, {                                                                        \
>> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
>> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
>> +    {},                                                                       \
>> +    {                                                                         \
>> +      1,          /* SizePropertyValid */                                     \
>> +      1,          /* NumberOfSetsValid */                                     \
>> +      1,          /* AssociativityValid */                                    \
>> +      0,          /* AllocationTypeValid */                                   \
>> +      1,          /* CacheTypeValid */                                        \
>> +      1,          /* WritePolicyValid */                                      \
>> +      1,          /* LineSizeValid */                                         \
>> +    },                                                                        \
>> +    0,            /* NextLevelOfCache */                                      \
>> +    SIZE_32KB,    /* Size */                                                  \
>> +    256,          /* NumberOfSets */                                          \
>> +    2,            /* Associativity */                                         \
>> +    {                                                                         \
>> +      0,                                                /* AllocationType */  \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                   \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
>> +    },                                                                        \
>> +    64            /* LineSize */                                              \
>> +  }                                                                           \
>> +}
>> +
>> +#define PPTT_CLUSTER(pid, cid) {                                              \
>> +  {                                                                           \
>> +    EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,                                         \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_CLUSTER, L2Cache),                           \
>> +    {},                                                                       \
>> +    {                                                                         \
>> +      0,                                      /* PhysicalPackage */           \
>> +      EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */      \
>> +    },                                                                        \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid]), /* Parent */          \
>> +    0,                                        /* AcpiProcessorId */           \
>> +    1,                                        /* NumberOfPrivateResources */  \
>> +  }, {                                                                        \
>> +    FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[pid].Clusters[cid].L2Cache), \
>> +  }, {                                                                        \
>> +    EFI_ACPI_6_2_PPTT_TYPE_CACHE,                                             \
>> +    sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),                               \
>> +    {},                                                                       \
>> +    {                                                                         \
>> +      1,          /* SizePropertyValid */                                     \
>> +      1,          /* NumberOfSetsValid */                                     \
>> +      1,          /* AssociativityValid */                                    \
>> +      0,          /* AllocationTypeValid */                                   \
>> +      1,          /* CacheTypeValid */                                        \
>> +      1,          /* WritePolicyValid */                                      \
>> +      1,          /* LineSizeValid */                                         \
>> +    },                                                                        \
>> +    0,            /* NextLevelOfCache */                                      \
>> +    SIZE_256KB,   /* Size */                                                  \
>> +    256,          /* NumberOfSets */                                          \
>> +    16,           /* Associativity */                                         \
>> +    {                                                                         \
>> +      0,                                                /* AllocationType */  \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                       \
>> +      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                  \
>> +    },                                                                        \
>> +    64            /* LineSize */                                              \
>> +  }, {                                                                        \
>> +    PPTT_CORE(pid, cid, 0),                                                   \
>> +    PPTT_CORE(pid, cid, 1),                                                   \
>> +  }                                                                           \
>> +}
>> +
>> +STATIC SYNQUACER_PPTT_TABLE mSynQuacerPpttTable = {
>> +  {
>> +    __ACPI_HEADER(EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
>> +                  SYNQUACER_PPTT_TABLE,
>> +                  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION),
>> +  },
>> +  {
>> +    {
>> +      {
>> +        EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,
>> +        FIELD_OFFSET (SYNQUACER_PPTT_PACKAGE, L3Cache),
>> +        {},
>> +        {
>> +          1,                                      /* PhysicalPackage */
>> +          EFI_ACPI_6_2_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */
>> +        },
>> +        0,                                        /* Parent */
>> +        0,                                        /* AcpiProcessorId */
>> +        1,                                        /* NumberOfPrivateResources */
>> +      }, {
>> +        FIELD_OFFSET (SYNQUACER_PPTT_TABLE, Packages[0].L3Cache),
>> +      }, {
>> +        EFI_ACPI_6_2_PPTT_TYPE_CACHE,
>> +        sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE),
>> +        {},
>> +        {
>> +          1,                                      /* SizePropertyValid */
>> +          1,                                      /* NumberOfSetsValid */
>> +          1,                                      /* AssociativityValid */
>> +          0,                                      /* AllocationTypeValid */
>> +          1,                                      /* CacheTypeValid */
>> +          1,                                      /* WritePolicyValid */
>> +          1,                                      /* LineSizeValid */
>> +        },
>> +        0,                                        /* NextLevelOfCache */
>> +        SIZE_4MB,                                 /* Size */
>> +        4096,                                     /* NumberOfSets */
>> +        16,                                       /* Associativity */
>> +        {
>> +          0,                                      /* AllocationType */
>> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,
>> +          EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,
>> +        },
>> +        64                                        /* LineSize */
>> +      }, {
>> +        PPTT_CLUSTER (0, 0),
>> +        PPTT_CLUSTER (0, 1),
>> +        PPTT_CLUSTER (0, 2),
>> +        PPTT_CLUSTER (0, 3),
>> +        PPTT_CLUSTER (0, 4),
>> +        PPTT_CLUSTER (0, 5),
>> +        PPTT_CLUSTER (0, 6),
>> +        PPTT_CLUSTER (0, 7),
>> +        PPTT_CLUSTER (0, 8),
>> +        PPTT_CLUSTER (0, 9),
>> +        PPTT_CLUSTER (0, 10),
>> +        PPTT_CLUSTER (0, 11),
>> +      }
>> +    }
>> +  }
>> +};
>> +
>> +VOID* CONST ReferenceAcpiTable = &mSynQuacerPpttTable;
>> --
>> 2.15.1
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-27 12:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-08 13:13 [PATCH edk2-platforms] Silicon/SynQuacer: add PPTT ACPI table to describe cache topology Ard Biesheuvel
2018-03-08 17:24 ` Jeremy Linton
2018-03-08 17:27   ` Ard Biesheuvel
2018-03-08 17:50     ` Jeremy Linton
2018-03-08 17:59       ` Ard Biesheuvel
2018-03-08 20:38         ` Ard Biesheuvel
2018-03-08 21:48           ` Jeremy Linton
2018-03-08 22:56             ` Ard Biesheuvel
2018-04-27 10:58 ` Leif Lindholm
2018-04-27 12:12   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox