* [edk2-devel] [PATCH 0/2] Align the PPTT tables with qemu configuration
@ 2024-04-17 11:26 Xiong Yining
2024-04-17 11:26 ` [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls Xiong Yining
2024-04-17 11:26 ` [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration Xiong Yining
0 siblings, 2 replies; 5+ messages in thread
From: Xiong Yining @ 2024-04-17 11:26 UTC (permalink / raw)
To: devel
Cc: quic_llindhol, ardb+tianocore, graeme, marcin.juszkiewicz,
chenbaozi, Xiong Yining
To align the CPU topology information recognized by the operating system
with the CPU topology information configured by QEMU, we need to get the
information of topology to have complex PPTT tables setups.
When creating the pptt table, we considered the cluster layer,
SBSA Reference Platform has cluster support on qemu
https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg02951.html.
We use the SMC to get CPU topology information, the TF-A part is here
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27189.
Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
Xiong Yining (2):
Platform/SbsaQemu: get the information of CPU topology via SMC calls
Silicon/SbsaQemu: align the PPTT tables with qemu configuration
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h | 12 ++
.../Include/IndustryStandard/SbsaQemuAcpi.h | 32 ----
.../Include/IndustryStandard/SbsaQemuSmc.h | 1 +
.../Include/Library/HardwareInfoLib.h | 26 ++++
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 140 +++++++++++++-----
.../SbsaQemuHardwareInfoLib.c | 33 +++++
6 files changed, 174 insertions(+), 70 deletions(-)
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117916): https://edk2.groups.io/g/devel/message/117916
Mute This Topic: https://groups.io/mt/105575037/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread
* [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls
2024-04-17 11:26 [edk2-devel] [PATCH 0/2] Align the PPTT tables with qemu configuration Xiong Yining
@ 2024-04-17 11:26 ` Xiong Yining
2024-06-21 8:29 ` Marcin Juszkiewicz
2024-04-17 11:26 ` [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration Xiong Yining
1 sibling, 1 reply; 5+ messages in thread
From: Xiong Yining @ 2024-04-17 11:26 UTC (permalink / raw)
To: devel
Cc: quic_llindhol, ardb+tianocore, graeme, marcin.juszkiewicz,
chenbaozi, Xiong Yining
Provide functions to check for CPU topology information:
- the number of sockets on sbsa-ref platform.
- the number of clusters in one socket.
- the number of cores in one cluster.
- the number of threads in one cores.
As SMC calls can return up to 4 return values. the number of
sockets, clusters and cores are read from TF-A using platform
specific SMC calls. And the number of threads is caluculated
using the total number of cpus and the number of sockets,
clusters and cores.
Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
---
.../Include/IndustryStandard/SbsaQemuSmc.h | 1 +
.../Include/Library/HardwareInfoLib.h | 26 +++++++++++++++
.../SbsaQemuHardwareInfoLib.c | 33 +++++++++++++++++++
3 files changed, 60 insertions(+)
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
index e3092007d27d..9d9780ca70fe 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
@@ -16,6 +16,7 @@
#define SIP_SVC_GET_GIC_ITS SMC_SIP_FUNCTION_ID(101)
#define SIP_SVC_GET_CPU_COUNT SMC_SIP_FUNCTION_ID(200)
#define SIP_SVC_GET_CPU_NODE SMC_SIP_FUNCTION_ID(201)
+#define SIP_SVC_GET_CPU_TOPOLOGY SMC_SIP_FUNCTION_ID(202)
#define SIP_SVC_GET_MEMORY_NODE_COUNT SMC_SIP_FUNCTION_ID(300)
#define SIP_SVC_GET_MEMORY_NODE SMC_SIP_FUNCTION_ID(301)
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
index 46fdad45353c..3e451ee344c7 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
@@ -15,6 +15,19 @@ typedef struct{
UINT64 AddressSize;
} MemoryInfo;
+/**
+ Sockets: the number of sockets on sbsa-ref platform.
+ Clusters: the number of clusters in one socket.
+ Cores: the number of cores in one cluster.
+ Threads: the number of threads in one core.
+**/
+typedef struct{
+ UINT32 Sockets;
+ UINT32 Clusters;
+ UINT32 Cores;
+ UINT32 Threads;
+} CpuTopology;
+
/**
Get CPU count from information passed by Qemu.
@@ -83,4 +96,17 @@ GetNumaNodeCount (
VOID
);
+/**
+ Get cpu topology(sockets, clusters, cores, threads) from device tree passed by Qemu.
+
+ @param [out] CpuTopo A pointer to the cpu topology.
+
+
+ @retval the information of cpu topology.
+**/
+VOID
+GetCpuTopology (
+ OUT CpuTopology *CpuTopo
+ );
+
#endif /* HARDWARE_INFO_LIB */
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index 4c22e7d6ee47..a12dc0244da5 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -179,3 +179,36 @@ GetNumaNodeCount (
return NumberNumaNodes;
}
+
+/**
+ Get CPU topology.
+**/
+VOID
+GetCpuTopology (
+ OUT CpuTopology *CpuTopo
+ )
+{
+ UINTN SmcResult;
+ UINTN Arg0;
+ UINTN Arg1;
+ UINTN Arg2;
+ UINT32 NumCores = GetCpuCount();
+
+ SmcResult = ArmCallSmc0(SIP_SVC_GET_CPU_TOPOLOGY, &Arg0, &Arg1, &Arg2);
+ if (SmcResult != SMC_SIP_CALL_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "%a: SIP_SVC_GET_CPU_TOPOLOGY call failed. We have no cpu topology information.\n", __FUNCTION__));
+ ResetShutdown ();
+ } else {
+ CpuTopo->Sockets = Arg0;
+ CpuTopo->Clusters = Arg1;
+ CpuTopo->Cores = Arg2;
+ CpuTopo->Threads = NumCores / (CpuTopo->Sockets * CpuTopo->Clusters * CpuTopo->Cores);
+ }
+
+ DEBUG(( DEBUG_INFO, "%a: CPU Topology: sockets are %d, clusters are %d, cores are %d, threads are %d\n",
+ __FUNCTION__,
+ CpuTopo->Sockets,
+ CpuTopo->Clusters,
+ CpuTopo->Cores,
+ CpuTopo->Threads ));
+}
\ No newline at end of file
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117915): https://edk2.groups.io/g/devel/message/117915
Mute This Topic: https://groups.io/mt/105575034/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration
2024-04-17 11:26 [edk2-devel] [PATCH 0/2] Align the PPTT tables with qemu configuration Xiong Yining
2024-04-17 11:26 ` [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls Xiong Yining
@ 2024-04-17 11:26 ` Xiong Yining
2024-06-21 8:29 ` Marcin Juszkiewicz
1 sibling, 1 reply; 5+ messages in thread
From: Xiong Yining @ 2024-04-17 11:26 UTC (permalink / raw)
To: devel
Cc: quic_llindhol, ardb+tianocore, graeme, marcin.juszkiewicz,
chenbaozi, Xiong Yining
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 can get the CPU topology information via SMC.
Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
---
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h | 12 ++
.../Include/IndustryStandard/SbsaQemuAcpi.h | 32 ----
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 140 +++++++++++++-----
3 files changed, 114 insertions(+), 70 deletions(-)
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
index 83a085cd86f4..e29635b28938 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
@@ -90,4 +90,16 @@ typedef struct {
ClockDomain /* Clock Domain */ \
}
+#define SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT( \
+ Flags, Parent, ACPIProcessorID, NumberOfPrivateResources) \
+ { \
+ EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR, /* Type */ \
+ sizeof (EFI_ACPI_6_3_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 61d8bce8c959..7ef85b7e2f79 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -166,36 +166,4 @@ typedef struct {
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 */ \
- }
-
#endif
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 30239e7dca0d..6e2258a3a54d 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -496,14 +496,23 @@ AddPpttTable (
EFI_PHYSICAL_ADDRESS PageAddress;
UINT8 *New;
UINT32 CpuId;
- UINT32 NumCores = GetCpuCount ();
+ CpuTopology CpuTopo;
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;
- 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_3_PPTT_STRUCTURE_PROCESSOR_FLAGS SocketFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID,
+ EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
+
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_FLAGS ClusterFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID,
+ EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
+
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_FLAGS CoreFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,
+ EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
+
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_FLAGS ThreadFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,
+ EFI_ACPI_6_3_PPTT_PROCESSOR_IS_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
EFI_ACPI_DESCRIPTION_HEADER Header =
SBSAQEMU_ACPI_HEADER (
@@ -511,11 +520,17 @@ AddPpttTable (
EFI_ACPI_DESCRIPTION_HEADER,
EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION);
+ GetCpuTopology(&CpuTopo);
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_3_PPTT_STRUCTURE_PROCESSOR) +
+ CpuTopo.Clusters * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
+ sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) * 3 +
+ CpuTopo.Cores * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
+ sizeof (UINT32) * 2)));
+
+ if (CpuTopo.Threads > 1){
+ TableSize += CpuTopo.Sockets * CpuTopo.Clusters * CpuTopo.Cores * CpuTopo.Threads * sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+ }
Status = gBS->AllocatePages (
AllocateAnyPages,
@@ -536,39 +551,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;
+ UINT32 SocketNum, ClusterNum, CoreNum, ThreadNum;
+ UINT32 SocketIndex, ClusterIndex, CoreIndex, L1DCacheIndex, L1ICacheIndex, L2CacheIndex;
+
+ CpuId = 0;
+ SocketIndex = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
+ for (SocketNum = 0; SocketNum < CpuTopo.Sockets; SocketNum++) {
+ // Add the Socket PPTT structure
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Socket = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(SocketFlags, 0, 0, 0);
+ CopyMem (New, &Socket, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
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));
+ ClusterIndex = SocketIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+ for(ClusterNum = 0; ClusterNum < CpuTopo.Clusters; ClusterNum++) {
+ L1DCacheIndex = ClusterIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+ L1ICacheIndex = L1DCacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+ L2CacheIndex = L1ICacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+ CoreIndex = L2CacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+ // Add the Cluster PPTT structure
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Cluster = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(ClusterFlags, SocketIndex , 0, 0);
+ 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 = L2CacheIndex;
+ 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 = L2CacheIndex;
+ New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+ // Add L2 Cache structure
+ CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
+ New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+ for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) {
+ UINT32 *PrivateResourcePtr;
+
+ if (CpuTopo.Threads > 1) {
+ // Add the Core PPTT structure. The Thread structure is the leaf structure, adjust the value of CoreFlags.
+ CoreFlags.AcpiProcessorIdValid = EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID;
+ CoreFlags.NodeIsALeaf = EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF;
+
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(CoreFlags, ClusterIndex, 0, 2);
+ CopyMem (New, &Core, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+ New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+
+ PrivateResourcePtr = (UINT32 *) New;
+ PrivateResourcePtr[0] = L1DCacheIndex;
+ PrivateResourcePtr[1] = L1ICacheIndex;
+ New += (2 * sizeof (UINT32));
+
+ // Add the Thread PPTT structure
+ for (ThreadNum = 0; ThreadNum < CpuTopo.Threads; ThreadNum++) {
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Thread = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(ThreadFlags, CoreIndex, CpuId, 0);
+ CopyMem (New, &Thread, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+ New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+ CpuId ++;
+ }
+
+ CoreIndex += CpuTopo.Threads * sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+ } else {
+ // Add the Core PPTT structure
+ EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(CoreFlags, ClusterIndex, CpuId, 2);
+ CopyMem (New, &Core, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+ New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+
+ PrivateResourcePtr = (UINT32 *) New;
+ PrivateResourcePtr[0] = L1DCacheIndex;
+ PrivateResourcePtr[1] = L1ICacheIndex;
+ New += (2 * sizeof (UINT32));
+ CpuId ++;
+ }
+
+ CoreIndex += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2;
+ }
+
+ ClusterIndex = CoreIndex;
+ }
+ SocketIndex = ClusterIndex;
}
// Perform Checksum
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117914): https://edk2.groups.io/g/devel/message/117914
Mute This Topic: https://groups.io/mt/105575033/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration
2024-04-17 11:26 ` [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration Xiong Yining
@ 2024-06-21 8:29 ` Marcin Juszkiewicz
0 siblings, 0 replies; 5+ messages in thread
From: Marcin Juszkiewicz @ 2024-06-21 8:29 UTC (permalink / raw)
To: Xiong Yining, devel; +Cc: quic_llindhol, ardb+tianocore, graeme, chenbaozi
W dniu 17.04.2024 o 13:26, Xiong Yining pisze:
> 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 can get the CPU topology information via SMC.
>
> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
Thanks for submittion. Some issues to handle. Also: please call
uncrucify on your code.
> ---
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h | 12 ++
> .../Include/IndustryStandard/SbsaQemuAcpi.h | 32 ----
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 140 +++++++++++++-----
> 3 files changed, 114 insertions(+), 70 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> index 83a085cd86f4..e29635b28938 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
> @@ -90,4 +90,16 @@ typedef struct {
> ClockDomain /* Clock Domain */ \
> }
>
> +#define SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT( \
> + Flags, Parent, ACPIProcessorID, NumberOfPrivateResources) \
> + { \
> + EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR, /* Type */ \
> + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + NumberOfPrivateResources * sizeof (UINT32), /* Length */ \
First suggestion: please use latest ACPI version (6.5 at the moment).
There are 2-3 entries more inside of struct. One of them is CacheId
which can be anything as long as it is unique and > 0 (just do
incremental).
> + { 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 61d8bce8c959..7ef85b7e2f79 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> @@ -166,36 +166,4 @@ typedef struct {
> 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 */ \
> - }
> -
> #endif
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 30239e7dca0d..6e2258a3a54d 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -496,14 +496,23 @@ AddPpttTable (
> EFI_PHYSICAL_ADDRESS PageAddress;
> UINT8 *New;
> UINT32 CpuId;
> - UINT32 NumCores = GetCpuCount ();
> + CpuTopology CpuTopo;
>
> 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;
>
> - 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_3_PPTT_STRUCTURE_PROCESSOR_FLAGS SocketFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID,
> + EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
Please switch to one flag per line - will be more readable:
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_3_PPTT_STRUCTURE_PROCESSOR_FLAGS ClusterFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID,
> + EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
> +
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_FLAGS CoreFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,
> + EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
Can you move "if threads > 1" mangling of CoreFlags here?
Will be at one place == more readable. May require reading
topology earlier.
> +
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR_FLAGS ThreadFlags = { EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,
> + EFI_ACPI_6_3_PPTT_PROCESSOR_IS_THREAD, EFI_ACPI_6_3_PPTT_NODE_IS_LEAF, EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, };
>
> EFI_ACPI_DESCRIPTION_HEADER Header =
> SBSAQEMU_ACPI_HEADER (
> @@ -511,11 +520,17 @@ AddPpttTable (
> EFI_ACPI_DESCRIPTION_HEADER,
> EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION);
>
> + GetCpuTopology(&CpuTopo);
> 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_3_PPTT_STRUCTURE_PROCESSOR) +
> + CpuTopo.Clusters * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
> + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) * 3 +
> + CpuTopo.Cores * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
> + sizeof (UINT32) * 2)));
Please reformat that part. Took me a while to check are numbers correct.
Use of uncrustify on a new code is a good move.
TableSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
CpuTopo.Sockets * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
CpuTopo.Clusters * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) * 3 +
CpuTopo.Cores * (sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
sizeof (UINT32) * 2)));
Speaking of caches. During Linaro Connect MAD24 I was asked to move
caches from Cluster level to Core level. This is to make MPAM
integration easier. Can you move caches to Cores?
> +
> + if (CpuTopo.Threads > 1){
> + TableSize += CpuTopo.Sockets * CpuTopo.Clusters * CpuTopo.Cores * CpuTopo.Threads * sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> + }
>
> Status = gBS->AllocatePages (
> AllocateAnyPages,
> @@ -536,39 +551,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;
> + UINT32 SocketNum, ClusterNum, CoreNum, ThreadNum;
> + UINT32 SocketIndex, ClusterIndex, CoreIndex, L1DCacheIndex, L1ICacheIndex, L2CacheIndex;
> +
> + CpuId = 0;
> + SocketIndex = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
> + for (SocketNum = 0; SocketNum < CpuTopo.Sockets; SocketNum++) {
> + // Add the Socket PPTT structure
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Socket = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(SocketFlags, 0, 0, 0);
> + CopyMem (New, &Socket, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> 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));
> + ClusterIndex = SocketIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> + for(ClusterNum = 0; ClusterNum < CpuTopo.Clusters; ClusterNum++) {
> + L1DCacheIndex = ClusterIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> + L1ICacheIndex = L1DCacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> + L2CacheIndex = L1ICacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> + CoreIndex = L2CacheIndex + sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> +
> + // Add the Cluster PPTT structure
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Cluster = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(ClusterFlags, SocketIndex , 0, 0);
> + 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
L1DCache.CacheId = CacheId++;
// will handle uniqueness of cache ids (just start from 1, not from 0).
// same for L1I and L2 caches
> + CopyMem (New, &L1DCache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
> + ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE*) New)->NextLevelOfCache = L2CacheIndex;
> + 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 = L2CacheIndex;
> + New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> +
> + // Add L2 Cache structure
> + CopyMem (New, &L2Cache, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
> + New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
> +
> + for (CoreNum = 0; CoreNum < CpuTopo.Cores; CoreNum++) {
> + UINT32 *PrivateResourcePtr;
> +
> + if (CpuTopo.Threads > 1) {
> + // Add the Core PPTT structure. The Thread structure is the leaf structure, adjust the value of CoreFlags.
> + CoreFlags.AcpiProcessorIdValid = EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID;
> + CoreFlags.NodeIsALeaf = EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF;
Move those 3 lines above - after defining CoreFlags.
That if/else block repeats lot of code. Can you change it?
> +
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(CoreFlags, ClusterIndex, 0, 2);
> + CopyMem (New, &Core, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> + New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> +
> + PrivateResourcePtr = (UINT32 *) New;
> + PrivateResourcePtr[0] = L1DCacheIndex;
> + PrivateResourcePtr[1] = L1ICacheIndex;
> + New += (2 * sizeof (UINT32));
> +
> + // Add the Thread PPTT structure
> + for (ThreadNum = 0; ThreadNum < CpuTopo.Threads; ThreadNum++) {
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Thread = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(ThreadFlags, CoreIndex, CpuId, 0);
> + CopyMem (New, &Thread, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> + New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> + CpuId ++;
> + }
> +
> + CoreIndex += CpuTopo.Threads * sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> + } else {
> + // Add the Core PPTT structure
> + EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(CoreFlags, ClusterIndex, CpuId, 2);
> + CopyMem (New, &Core, sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
> + New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
> +
> + PrivateResourcePtr = (UINT32 *) New;
> + PrivateResourcePtr[0] = L1DCacheIndex;
> + PrivateResourcePtr[1] = L1ICacheIndex;
> + New += (2 * sizeof (UINT32));
> + CpuId ++;
> + }
> +
> + CoreIndex += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + sizeof (UINT32) * 2;
> + }
> +
> + ClusterIndex = CoreIndex;
> + }
> + SocketIndex = ClusterIndex;
> }
>
> // Perform Checksum
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119667): https://edk2.groups.io/g/devel/message/119667
Mute This Topic: https://groups.io/mt/105575033/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls
2024-04-17 11:26 ` [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls Xiong Yining
@ 2024-06-21 8:29 ` Marcin Juszkiewicz
0 siblings, 0 replies; 5+ messages in thread
From: Marcin Juszkiewicz @ 2024-06-21 8:29 UTC (permalink / raw)
To: Xiong Yining, devel; +Cc: quic_llindhol, ardb+tianocore, graeme, chenbaozi
W dniu 17.04.2024 o 13:26, Xiong Yining pisze:
> Provide functions to check for CPU topology information:
> - the number of sockets on sbsa-ref platform.
> - the number of clusters in one socket.
> - the number of cores in one cluster.
> - the number of threads in one cores.
>
> As SMC calls can return up to 4 return values. the number of
> sockets, clusters and cores are read from TF-A using platform
> specific SMC calls. And the number of threads is caluculated
> using the total number of cpus and the number of sockets,
> clusters and cores.
>
> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
> ---
> .../Include/IndustryStandard/SbsaQemuSmc.h | 1 +
> .../Include/Library/HardwareInfoLib.h | 26 +++++++++++++++
> .../SbsaQemuHardwareInfoLib.c | 33 +++++++++++++++++++
> 3 files changed, 60 insertions(+)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> index e3092007d27d..9d9780ca70fe 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> @@ -16,6 +16,7 @@
> #define SIP_SVC_GET_GIC_ITS SMC_SIP_FUNCTION_ID(101)
> #define SIP_SVC_GET_CPU_COUNT SMC_SIP_FUNCTION_ID(200)
> #define SIP_SVC_GET_CPU_NODE SMC_SIP_FUNCTION_ID(201)
> +#define SIP_SVC_GET_CPU_TOPOLOGY SMC_SIP_FUNCTION_ID(202)
> #define SIP_SVC_GET_MEMORY_NODE_COUNT SMC_SIP_FUNCTION_ID(300)
> #define SIP_SVC_GET_MEMORY_NODE SMC_SIP_FUNCTION_ID(301)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> index 46fdad45353c..3e451ee344c7 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
> @@ -15,6 +15,19 @@ typedef struct{
> UINT64 AddressSize;
> } MemoryInfo;
>
> +/**
> + Sockets: the number of sockets on sbsa-ref platform.
> + Clusters: the number of clusters in one socket.
> + Cores: the number of cores in one cluster.
> + Threads: the number of threads in one core.
> +**/
> +typedef struct{
> + UINT32 Sockets;
> + UINT32 Clusters;
> + UINT32 Cores;
> + UINT32 Threads;
> +} CpuTopology;
> +
> /**
> Get CPU count from information passed by Qemu.
>
> @@ -83,4 +96,17 @@ GetNumaNodeCount (
> VOID
> );
>
> +/**
> + Get cpu topology(sockets, clusters, cores, threads) from device tree passed by Qemu.
> +
> + @param [out] CpuTopo A pointer to the cpu topology.
> +
> +
> + @retval the information of cpu topology.
> +**/
> +VOID
> +GetCpuTopology (
> + OUT CpuTopology *CpuTopo
> + );
> +
> #endif /* HARDWARE_INFO_LIB */
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> index 4c22e7d6ee47..a12dc0244da5 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -179,3 +179,36 @@ GetNumaNodeCount (
>
> return NumberNumaNodes;
> }
> +
> +/**
> + Get CPU topology.
> +**/
> +VOID
> +GetCpuTopology (
> + OUT CpuTopology *CpuTopo
> + )
> +{
> + UINTN SmcResult;
> + UINTN Arg0;
> + UINTN Arg1;
> + UINTN Arg2;
> + UINT32 NumCores = GetCpuCount();
> +
> + SmcResult = ArmCallSmc0(SIP_SVC_GET_CPU_TOPOLOGY, &Arg0, &Arg1, &Arg2);
Can you use ArmCallSmc() directly? TF-A returns 5 arguments for this SMC
call, we use 4 and calculate 5th one. Once we improve SMCCC code in EDK2
the 5th one would be used directly.
> + if (SmcResult != SMC_SIP_CALL_SUCCESS) {
> + DEBUG ((DEBUG_ERROR, "%a: SIP_SVC_GET_CPU_TOPOLOGY call failed. We have no cpu topology information.\n", __FUNCTION__));
> + ResetShutdown ();
> + } else {
> + CpuTopo->Sockets = Arg0;
> + CpuTopo->Clusters = Arg1;
> + CpuTopo->Cores = Arg2;
> + CpuTopo->Threads = NumCores / (CpuTopo->Sockets * CpuTopo->Clusters * CpuTopo->Cores);
Once SMCCC gets improved this would be read directly from TF-A.
> + }
> +
> + DEBUG(( DEBUG_INFO, "%a: CPU Topology: sockets are %d, clusters are %d, cores are %d, threads are %d\n",
Can you replace " are" with ":" ("sockets: %d, clusters: %d")? Those
numbers can be singular.
> + __FUNCTION__,
> + CpuTopo->Sockets,
> + CpuTopo->Clusters,
> + CpuTopo->Cores,
> + CpuTopo->Threads ));
> +}
> \ No newline at end of file
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119668): https://edk2.groups.io/g/devel/message/119668
Mute This Topic: https://groups.io/mt/105575034/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-21 8:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17 11:26 [edk2-devel] [PATCH 0/2] Align the PPTT tables with qemu configuration Xiong Yining
2024-04-17 11:26 ` [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls Xiong Yining
2024-06-21 8:29 ` Marcin Juszkiewicz
2024-04-17 11:26 ` [edk2-devel] [PATCH 2/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration Xiong Yining
2024-06-21 8:29 ` Marcin Juszkiewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox