public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <quic_llindhol@quicinc.com>
To: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Cc: <devel@edk2.groups.io>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Graeme Gregory <graeme@xora.org.uk>,
	Chen Baozi <chenbaozi@phytium.com.cn>,
	Xiong Yining <xiongyining1480@phytium.com.cn>
Subject: Re: [edk2-devel] [PATCH edk2-platforms v2 2/3] Silicon/SbsaQemu: align the PPTT tables with QEMU
Date: Thu, 4 Jul 2024 12:57:51 +0100	[thread overview]
Message-ID: <ZoaOP3WtUFmUiiew@qc-i7.hemma.eciton.net> (raw)
In-Reply-To: <20240702-acpi65-v2-2-3cb18a892221@linaro.org>

On Tue, Jul 02, 2024 at 18:33:03 +0200, Marcin Juszkiewicz wrote:
> From: Xiong Yining <xiongyining1480@phytium.com.cn>
> 
> To align the CPU topology information recognized by the operating system
> with the CPU topology information configured by QEMU, we need to make
> use of the CPU topology information to create complex PPTT tables
> setups.
> 
> We get the CPU topology information via SMC.
> 
> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h       |  17 +-
>  .../Include/IndustryStandard/SbsaQemuAcpi.h         |  78 +++-----
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c       | 190 +++++++++++++++-----
>  3 files changed, 180 insertions(+), 105 deletions(-)
> 
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> index e5f0748bb16e..5e50749051c9 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> @@ -61,8 +61,7 @@ typedef struct {
>  
>  #define GTDT_WDTIMER_FLAGS  (GTDT_WDTIMER_ACTIVE_HIGH | GTDT_WDTIMER_LEVEL_TRIGGERED)
>  
> -#define SBSAQEMU_ACPI_MEMORY_AFFINITY_STRUCTURE_INIT(                             \
> -                                                                                  ProximityDomain, Base, Length, Flags)                                   \
> +#define SBSAQEMU_ACPI_MEMORY_AFFINITY_STRUCTURE_INIT(ProximityDomain, Base, Length, Flags) \

Please separate functional and non-functional changes.

>    {                                                                               \
>      1,                                                  /* Type */                \
>      sizeof (EFI_ACPI_6_4_MEMORY_AFFINITY_STRUCTURE),    /* Length */              \
> @@ -77,8 +76,7 @@ typedef struct {
>      0                                                   /* Reserved */            \
>    }
>  
> -#define SBSAQEMU_ACPI_GICC_AFFINITY_STRUCTURE_INIT(                               \
> -                                                                                  ProximityDomain, ACPIProcessorUID, Flags, ClockDomain)                  \
> +#define SBSAQEMU_ACPI_GICC_AFFINITY_STRUCTURE_INIT(ProximityDomain, ACPIProcessorUID, Flags, ClockDomain) \

And again.

>    {                                                                               \
>      3,                                                  /* Type */                \
>      sizeof (EFI_ACPI_6_4_GICC_AFFINITY_STRUCTURE),      /* Length */              \
> @@ -88,4 +86,15 @@ typedef struct {
>      ClockDomain                                         /* Clock Domain */        \
>    }
>  
> +#define SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(Flags, Parent, ACPIProcessorID, NumberOfPrivateResources)             \
> +  {                                                                                                                                 \
> +    EFI_ACPI_6_5_PPTT_TYPE_PROCESSOR,                                                            /* Type */                         \
> +    sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) + NumberOfPrivateResources * sizeof (UINT32), /* Length */                       \
> +    { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                                          /* Reserved */                     \
> +    Flags,                                                                                       /* Flags */                        \
> +    Parent,                                                                                      /* Parent */                       \
> +    ACPIProcessorID,                                                                             /* ACPI Processor ID */            \
> +    NumberOfPrivateResources                                                                     /* Number of private resources */  \
> +  }
> +
>  #endif
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> index ae151210c2c6..fa2e2b30bb7d 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> @@ -87,13 +87,13 @@ typedef struct {
>  #define SBSAQEMU_L2_CACHE_ASSC  8
>  
>  #define CLUSTER_INDEX     (sizeof (EFI_ACPI_DESCRIPTION_HEADER))
> -#define L1_D_CACHE_INDEX  (CLUSTER_INDEX + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR))
> -#define L1_I_CACHE_INDEX  (L1_D_CACHE_INDEX + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE))
> -#define L2_CACHE_INDEX    (L1_I_CACHE_INDEX + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE))
> +#define L1_D_CACHE_INDEX  (CLUSTER_INDEX + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR))
> +#define L1_I_CACHE_INDEX  (L1_D_CACHE_INDEX + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE))
> +#define L2_CACHE_INDEX    (L1_I_CACHE_INDEX + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE))

A lot of these changes are just ACPI62/ACPI63->ACPI65. Could/should
that be a separate commit?

>  
>  #define SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT  {                                \
> -    EFI_ACPI_6_3_PPTT_TYPE_CACHE,                                              \
> -    sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE),                                \
> +    EFI_ACPI_6_5_PPTT_TYPE_CACHE,                                              \
> +    sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE),                                \
>      { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                        \
>      {                                                                          \
>        1,                       /* SizePropertyValid */                         \
> @@ -103,22 +103,24 @@ typedef struct {
>        1,                       /* CacheTypeValid */                            \
>        1,                       /* WritePolicyValid */                          \
>        1,                       /* LineSizeValid */                             \
> +      1,                       /* CacheIdValid */                              \
>      },                                                                         \
>      0,                         /* NextLevelOfCache */                          \
>      SBSAQEMU_L1_D_CACHE_SIZE,  /* Size */                                      \
>      SBSAQEMU_L1_D_CACHE_SETS,  /* NumberOfSets */                              \
>      SBSAQEMU_L1_D_CACHE_ASSC,  /* Associativity */                             \
>      {                                                                          \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_ALLOCATION_READ_WRITE,                     \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                           \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                   \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_ALLOCATION_READ_WRITE,                     \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_CACHE_TYPE_DATA,                           \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                   \
>      },                                                                         \
> -    64                         /* LineSize */                                  \
> +    64,                        /* LineSize */                                  \
> +    0                          /* CacheId */                                   \
>    }
>  
>  #define SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT  {                                \
> -    EFI_ACPI_6_3_PPTT_TYPE_CACHE,                                              \
> -    sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE),                                \
> +    EFI_ACPI_6_5_PPTT_TYPE_CACHE,                                              \
> +    sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE),                                \
>      { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                        \
>      {                                                                          \
>        1,                       /* SizePropertyValid */                         \
> @@ -128,22 +130,24 @@ typedef struct {
>        1,                       /* CacheTypeValid */                            \
>        1,                       /* WritePolicyValid */                          \
>        1,                       /* LineSizeValid */                             \
> +      1,                       /* CacheIdValid */                              \
>      },                                                                         \
>      0,                         /* NextLevelOfCache */                          \
>      SBSAQEMU_L1_I_CACHE_SIZE,  /* Size */                                      \
>      SBSAQEMU_L1_I_CACHE_SETS,  /* NumberOfSets */                              \
>      SBSAQEMU_L1_I_CACHE_ASSC,  /* Associativity */                             \
>      {                                                                          \
> -      EFI_ACPI_6_3_CACHE_ATTRIBUTES_ALLOCATION_READ,                           \
> -      EFI_ACPI_6_3_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                    \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_ALLOCATION_READ,                           \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_CACHE_TYPE_INSTRUCTION,                    \
>        0,                                                                       \
>      },                                                                         \
> -    64                         /* LineSize */                                  \
> +    64,                        /* LineSize */                                  \
> +    0                          /* CacheId */                                   \
>    }
>  
>  #define SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT  {                                  \
> -    EFI_ACPI_6_3_PPTT_TYPE_CACHE,                                              \
> -    sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE),                                \
> +    EFI_ACPI_6_5_PPTT_TYPE_CACHE,                                              \
> +    sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE),                                \
>      { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                        \
>      {                                                                          \
>        1,                     /* SizePropertyValid */                           \
> @@ -153,49 +157,19 @@ typedef struct {
>        1,                     /* CacheTypeValid */                              \
>        1,                     /* WritePolicyValid */                            \
>        1,                     /* LineSizeValid */                               \
> +      1,                     /* CacheIdValid */                                \
>      },                                                                         \
>      0,                       /* NextLevelOfCache */                            \
>      SBSAQEMU_L2_CACHE_SIZE,  /* Size */                                        \
>      SBSAQEMU_L2_CACHE_SETS,  /* NumberOfSets */                                \
>      SBSAQEMU_L2_CACHE_ASSC,  /* Associativity */                               \
>      {                                                                          \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_ALLOCATION_READ_WRITE,                     \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                        \
> -      EFI_ACPI_6_2_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                   \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_ALLOCATION_READ_WRITE,                     \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_CACHE_TYPE_UNIFIED,                        \
> +      EFI_ACPI_6_5_CACHE_ATTRIBUTES_WRITE_POLICY_WRITE_BACK,                   \
>      },                                                                         \
> -    64            /* LineSize */                                               \
> -  }
> -
> -#define SBSAQEMU_ACPI_PPTT_CLUSTER_STRUCT  {                                   \
> -    EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,                                          \
> -    sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR),                            \
> -    { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                        \
> -    {                                                                          \
> -      EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL,         /* PhysicalPackage */        \
> -      EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID,     /* AcpiProcessorIdValid */   \
> -      EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD,  /* Is not a Thread */        \
> -      EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF,         /* Not Leaf */               \
> -      EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, /* Identical Cores */        \
> -    },                                                                         \
> -    0,                                        /* Parent */                     \
> -    0,                                        /* AcpiProcessorId */            \
> -    0,                                        /* NumberOfPrivateResources */   \
> -  }
> -
> -#define SBSAQEMU_ACPI_PPTT_CORE_STRUCT  {                                      \
> -    EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,                                          \
> -    (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + (2 * sizeof (UINT32))),  \
> -    { EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },                        \
> -    {                                                                          \
> -      EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL,     /* PhysicalPackage */        \
> -      EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,       /* AcpiProcessorValid */     \
> -      EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD,  /* Is not a Thread */        \
> -      EFI_ACPI_6_3_PPTT_NODE_IS_LEAF,             /* Leaf */                   \
> -      EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, /* Identical Cores */        \
> -    },                                                                         \
> -    0,                                        /* Parent */                     \
> -    0,                                        /* AcpiProcessorId */            \
> -    2,                                        /* NumberOfPrivateResources */   \
> +    64,                      /* LineSize */                                    \
> +    0                        /* CacheId */                                     \
>    }
>  
>  #endif
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index e0eef54ff907..4c275faf7de6 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -505,27 +505,70 @@ AddPpttTable (
>    EFI_PHYSICAL_ADDRESS  PageAddress;
>    UINT8                 *New;
>    UINT32                CpuId;
> -  UINT32                NumCores = GetCpuCount ();
> +  CpuTopology           CpuTopo;
> +  UINT32                CacheId;
>  
> -  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE  L1DCache = SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT;
> -  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE  L1ICache = SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT;
> -  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE  L2Cache  = SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT;
> +  GetCpuTopology (&CpuTopo);
>  
> -  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  Cluster = SBSAQEMU_ACPI_PPTT_CLUSTER_STRUCT;
> -  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  Core    = SBSAQEMU_ACPI_PPTT_CORE_STRUCT;
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE  L1DCache = SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT;
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE  L1ICache = SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT;
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE  L2Cache  = SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT;
>  
> -  EFI_ACPI_DESCRIPTION_HEADER  Header =
> -    SBSAQEMU_ACPI_HEADER (
> -      EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> -      EFI_ACPI_DESCRIPTION_HEADER,
> -      EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION
> -      );
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS  SocketFlags = {
> +    EFI_ACPI_6_5_PPTT_PACKAGE_PHYSICAL,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_IS_NOT_THREAD,
> +    EFI_ACPI_6_5_PPTT_NODE_IS_NOT_LEAF,
> +    EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL
> +  };
> +
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS  ClusterFlags = {
> +    EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_IS_NOT_THREAD,
> +    EFI_ACPI_6_5_PPTT_NODE_IS_NOT_LEAF,
> +    EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL
> +  };
> +
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS  CoreFlags = {
> +    EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_IS_NOT_THREAD,
> +    EFI_ACPI_6_5_PPTT_NODE_IS_LEAF,
> +    EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL
> +  };
> +
> +  if (CpuTopo.Threads > 1) {
> +    // The Thread structure is the leaf structure, adjust the value of CoreFlags.
> +    CoreFlags.AcpiProcessorIdValid = EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID;
> +    CoreFlags.NodeIsALeaf          = EFI_ACPI_6_5_PPTT_NODE_IS_NOT_LEAF;
> +  }
> +
> +  EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS  ThreadFlags = {
> +    EFI_ACPI_6_5_PPTT_PACKAGE_NOT_PHYSICAL,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_ID_VALID,
> +    EFI_ACPI_6_5_PPTT_PROCESSOR_IS_THREAD,
> +    EFI_ACPI_6_5_PPTT_NODE_IS_LEAF,
> +    EFI_ACPI_6_5_PPTT_IMPLEMENTATION_IDENTICAL
> +  };
> +
> +  EFI_ACPI_DESCRIPTION_HEADER  Header = SBSAQEMU_ACPI_HEADER (
> +                                          EFI_ACPI_6_5_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> +                                          EFI_ACPI_DESCRIPTION_HEADER,
> +                                          EFI_ACPI_6_5_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION
> +                                          );
>  
>    TableSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
> -              sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
> -              (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) * 3) +
> -              (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) * NumCores) +
> -              (sizeof (UINT32) * 2 * NumCores);
> +              CpuTopo.Sockets * (sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) +
> +                                 CpuTopo.Clusters * (sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) +
> +                                                     sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE) * 3 +
> +                                                     CpuTopo.Cores * (sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) +
> +                                                                      sizeof (UINT32) * 2)));
> +
> +  if (CpuTopo.Threads > 1) {
> +    TableSize += CpuTopo.Sockets * CpuTopo.Clusters * CpuTopo.Cores * CpuTopo.Threads *
> +                 sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +  }
>  
>    Status = gBS->AllocatePages (
>                    AllocateAnyPages,
> @@ -546,39 +589,88 @@ AddPpttTable (
>    ((EFI_ACPI_DESCRIPTION_HEADER *)New)->Length = TableSize;
>    New                                         += sizeof (EFI_ACPI_DESCRIPTION_HEADER);
>  
> -  // Add the Cluster PPTT structure
> -  CopyMem (New, &Cluster, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> -  New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> -
> -  // Add L1 D Cache structure
> -  CopyMem (New, &L1DCache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
> -  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2_CACHE_INDEX;
> -  New                                                         += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> -
> -  // Add L1 I Cache structure
> -  CopyMem (New, &L1ICache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
> -  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2_CACHE_INDEX;
> -  New                                                         += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> -
> -  // Add L2 Cache structure
> -  CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
> -  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = 0; /* L2 is LLC */
> -  New                                                         += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> -
> -  for (CpuId = 0; CpuId < NumCores; CpuId++) {
> -    EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR  *CorePtr;
> -    UINT32                                 *PrivateResourcePtr;
> -
> -    CopyMem (New, &Core, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> -    CorePtr                  = (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *)New;
> -    CorePtr->Parent          = CLUSTER_INDEX;
> -    CorePtr->AcpiProcessorId = CpuId;
> -    New                     += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> -
> -    PrivateResourcePtr    = (UINT32 *)New;
> -    PrivateResourcePtr[0] = L1_D_CACHE_INDEX;
> -    PrivateResourcePtr[1] = L1_I_CACHE_INDEX;
> -    New                  += (2 * sizeof (UINT32));
> +  UINT32  SocketNum, ClusterNum, CoreNum, ThreadNum;
> +  UINT32  SocketIndex, ClusterIndex, CoreIndex, L1DCacheIndex, L1ICacheIndex, L2CacheIndex;
> +
> +  CpuId       = 0;
> +  CacheId     = 1; // 0 is not a valid Cache ID.
> +  SocketIndex = sizeof (EFI_ACPI_DESCRIPTION_HEADER);

These new variables added in the middle of a function, and then four
levels of for-loops with conditional levels on top of that:
could this block be moved to a helper function (or two)?

/
    Leif

> +
> +  for (SocketNum = 0; SocketNum < CpuTopo.Sockets; SocketNum++) {
> +    // Add the Socket PPTT structure
> +    EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR  Socket = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT (SocketFlags, 0, 0, 0);
> +    CopyMem (New, &Socket, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR));
> +    New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +
> +    ClusterIndex = SocketIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +    for (ClusterNum = 0; ClusterNum < CpuTopo.Clusters; ClusterNum++) {
> +      L1DCacheIndex = ClusterIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +      L1ICacheIndex = L1DCacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +      L2CacheIndex  = L1ICacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +      CoreIndex     = L2CacheIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> +      // Add the Cluster PPTT structure
> +      EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR  Cluster = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT (ClusterFlags, SocketIndex, 0, 0);
> +      CopyMem (New, &Cluster, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR));
> +      New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +
> +      L1DCache.CacheId = CacheId++;
> +      // Add L1 D Cache structure
> +      CopyMem (New, &L1DCache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE));
> +      ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex;
> +      New                                                         += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> +      L1ICache.CacheId = CacheId++;
> +      // Add L1 I Cache structure
> +      CopyMem (New, &L1ICache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE));
> +      ((EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE *)New)->NextLevelOfCache = L2CacheIndex;
> +      New                                                         += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> +      L2Cache.CacheId = CacheId++;
> +      // Add L2 Cache structure
> +      CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE));
> +      New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> +      for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) {
> +        UINT32  *PrivateResourcePtr;
> +        UINT32  CoreCpuId;
> +
> +        if (CpuTopo.Threads == 1) {
> +          CoreCpuId = CpuId;
> +        } else {
> +          CoreCpuId = 0;
> +        }
> +
> +        EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR  Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT (CoreFlags, ClusterIndex, CoreCpuId, 2);
> +        CopyMem (New, &Core, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR));
> +        New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +
> +        PrivateResourcePtr    = (UINT32 *)New;
> +        PrivateResourcePtr[0] = L1DCacheIndex;
> +        PrivateResourcePtr[1] = L1ICacheIndex;
> +        New                  += (2 * sizeof (UINT32));
> +
> +        if (CpuTopo.Threads == 1) {
> +          CpuId++;
> +        } else {
> +          // Add the Thread PPTT structure
> +          for (ThreadNum = 0; ThreadNum < CpuTopo.Threads; ThreadNum++) {
> +            EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR  Thread = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT (ThreadFlags, CoreIndex, CpuId, 0);
> +            CopyMem (New, &Thread, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR));
> +            New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +            CpuId++;
> +          }
> +
> +          CoreIndex +=  CpuTopo.Threads * sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +        }
> +
> +        CoreIndex += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2;
> +      }
> +
> +      ClusterIndex = CoreIndex;
> +    }
> +
> +    SocketIndex = ClusterIndex;
>    }
>  
>    // Perform Checksum
> 
> -- 
> 2.45.2
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119795): https://edk2.groups.io/g/devel/message/119795
Mute This Topic: https://groups.io/mt/107003194/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2024-07-04 11:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-02 16:33 [edk2-devel] [PATCH edk2-platforms v2 0/3] SbsaQemu: Align the PPTT tables with QEMU Marcin Juszkiewicz
2024-07-02 16:33 ` [edk2-devel] [PATCH edk2-platforms v2 1/3] Platform/SbsaQemu: get the information of CPU topology via SMC calls Marcin Juszkiewicz
2024-07-02 16:33 ` [edk2-devel] [PATCH edk2-platforms v2 2/3] Silicon/SbsaQemu: align the PPTT tables with QEMU Marcin Juszkiewicz
2024-07-04 11:57   ` Leif Lindholm [this message]
2024-07-02 16:33 ` [edk2-devel] [PATCH edk2-platforms v2 3/3] SbsaQemu: provide cache info per core in PPTT Marcin Juszkiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZoaOP3WtUFmUiiew@qc-i7.hemma.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox