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 <marcin.juszkiew...@linaro.org> > --- > .../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: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-