On Tue, Jul 09, 2024 at 12:47:10 +0200, Marcin Juszkiewicz wrote:
> Function AddPpttTable() adding PPTT got too long. This change moves part
> of it into helper function AddCoresToPpttTable() which takes care of
> generating entries for Core and below (Cache, Thread).
>
> Signed-off-by: Marcin Juszkiewicz <[email protected]>
> ---
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 237
> +++++++++++---------
> 1 file changed, 133 insertions(+), 104 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index a7a9664abdcb..a4b2ee2fdcb0 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -29,6 +29,9 @@
>
> static UINTN GicItsBase;
>
> +static UINTN CpuId;
> +static UINTN CacheId;
> +
static variables are supposed to have g (global) or m (module) prefix.
This is local, so m.
(Yes, that means I missed that when reviewing the GitIts bits.)
Also, why are these in a #pragma pack(1) block?
> #pragma pack ()
>
> /*
> @@ -491,6 +494,126 @@ AddSsdtTable (
> return Status;
> }
>
STATIC
> +UINT32
> +AddCoresToPpttTable (
> + UINT8 *New,
> + UINT32 ClusterIndex,
> + CpuTopology CpuTopo
> + )
> +{
> + UINT32 L1DCacheIndex;
> + UINT32 L1ICacheIndex;
> + UINT32 L2CacheIndex;
> + UINT32 CoreIndex;
> + UINT32 Index;
> + UINT32 CoreCpuId;
> + UINT32 CoreNum;
> + UINT32 ThreadNum;
> + UINT32 *PrivateResourcePtr;
> +
> + 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_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;
> +
> + CoreIndex = ClusterIndex + sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> + Index = CoreIndex;
> +
> + for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) {
> + if (CpuTopo.Threads == 1) {
> + CoreCpuId = CpuId;
> + } else {
> + CoreCpuId = 0;
> + }
> +
> + // space for Core + PrivateResourcePtr
> + Index += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> + Index += sizeof (UINT32) * 2;
> +
> + L1DCacheIndex = Index;
> + L1ICacheIndex = L1DCacheIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> + L2CacheIndex = L1ICacheIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> + 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));
> +
> + // Add L1 D Cache structure
> + L1DCache.CacheId = CacheId++;
> + 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);
> +
> + // Add L1 I Cache structure
> + L1ICache.CacheId = CacheId++;
> + 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);
> +
> + // Add L2 Cache structure
> + L2Cache.CacheId = CacheId++;
> + CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE));
> + New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> +
> + Index += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE) * 3;
> +
> + 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++;
> + }
> +
> + Index += CpuTopo.Threads * sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> + }
> +
> + CoreIndex = Index;
> + }
> +
> + return CoreIndex - ClusterIndex - sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> +}
> +
> /*
> * A function that adds the PPTT ACPI table.
> */
> @@ -502,18 +625,17 @@ AddPpttTable (
> EFI_STATUS Status;
> UINTN TableHandle;
> UINT32 TableSize;
> + UINT32 CoresPartSize;
> + UINT32 SocketNum;
> + UINT32 ClusterNum;
> + UINT32 SocketIndex;
> + UINT32 ClusterIndex;
> EFI_PHYSICAL_ADDRESS PageAddress;
> UINT8 *New;
> - UINT32 CpuId;
> CpuTopology CpuTopo;
> - UINT32 CacheId;
>
> GetCpuTopology (&CpuTopo);
>
> - 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_6_5_PPTT_STRUCTURE_PROCESSOR_FLAGS SocketFlags = {
> EFI_ACPI_6_5_PPTT_PACKAGE_PHYSICAL,
> EFI_ACPI_6_5_PPTT_PROCESSOR_ID_INVALID,
> @@ -530,28 +652,6 @@ AddPpttTable (
> 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,
> @@ -590,11 +690,9 @@ AddPpttTable (
> ((EFI_ACPI_DESCRIPTION_HEADER *)New)->Length = TableSize;
> New += sizeof
> (EFI_ACPI_DESCRIPTION_HEADER);
>
> - UINT32 SocketNum, ClusterNum, CoreNum, ThreadNum;
> - UINT32 SocketIndex, ClusterIndex, CoreIndex, L1DCacheIndex,
> L1ICacheIndex, L2CacheIndex;
> + CpuId = 0;
> + CacheId = 1; // 0 is not a valid Cache ID.
>
> - CpuId = 0;
> - CacheId = 1; // 0 is not a valid Cache ID.
> SocketIndex = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
> for (SocketNum = 0; SocketNum < CpuTopo.Sockets; SocketNum++) {
> // Add the Socket PPTT structure
> @@ -609,8 +707,6 @@ AddPpttTable (
>
> ClusterIndex = SocketIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> for (ClusterNum = 0; ClusterNum < CpuTopo.Clusters; ClusterNum++) {
> - CoreIndex = ClusterIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
> -
> // Add the Cluster PPTT structure
> EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR Cluster =
> SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT (
> ClusterFlags,
> @@ -621,76 +717,9 @@ AddPpttTable (
> CopyMem (New, &Cluster, sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR));
> New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR);
>
> - for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) {
> - UINT32 *PrivateResourcePtr;
> - UINT32 CoreCpuId;
> -
> - // two UINT32s for PrivateResourcePtr data
> - L1DCacheIndex = CoreIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2;
> - L1ICacheIndex = L1DCacheIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> - L2CacheIndex = L1ICacheIndex + sizeof
> (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> -
> - 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));
> -
> - // Add L1 D Cache structure
> - L1DCache.CacheId = CacheId++;
> - 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);
> -
> - // Add L1 I Cache structure
> - L1ICache.CacheId = CacheId++;
> - 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);
> -
> - // Add L2 Cache structure
> - L2Cache.CacheId = CacheId++;
> - CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE));
> - New += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE);
> -
> - 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;
> - CoreIndex += sizeof (EFI_ACPI_6_5_PPTT_STRUCTURE_CACHE) * 3;
> - }
> -
> - ClusterIndex = CoreIndex;
> + CoresPartSize = AddCoresToPpttTable (New, ClusterIndex, CpuTopo);
> + ClusterIndex += CoresPartSize;
This sounds like ClisterIndex is no longer an Index after this patch.
Should it be renamed?
/
Leif
> + New += CoresPartSize;
> }
>
> SocketIndex = ClusterIndex;
>
> --
> 2.45.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119841): https://edk2.groups.io/g/devel/message/119841
Mute This Topic: https://groups.io/mt/107120147/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
