[Skiboot] [PATCH] core/cpu: Fix invalid SMT indexes
Nicholas Piggin
npiggin at gmail.com
Mon Nov 11 10:41:15 AEDT 2024
Code using cpu_thread_count is dangerous because that is the maximum
number of threads that a CPU type supports, not the actual number of
threads. For real hardware that hardly matters, but QEMU can run a
single thread Power10, for example. This causes some code (e.g.,
xive_init_cpu_properties) to access beyond the end of allocated
arrays.
Fix this by making cpu_thread_count the actual number of threads
discovered via dt.
Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
---
core/cpu.c | 60 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/core/cpu.c b/core/cpu.c
index 48ee1a5cd..5713a512f 100644
--- a/core/cpu.c
+++ b/core/cpu.c
@@ -31,6 +31,7 @@ struct cpu_stack {
} __align(STACK_SIZE);
static struct cpu_stack * const cpu_stacks = (struct cpu_stack *)CPU_STACKS_BASE;
+static unsigned int cpu_threads_max;
unsigned int cpu_thread_count;
unsigned int cpu_max_pir;
struct cpu_thread *boot_cpu;
@@ -1063,29 +1064,29 @@ void init_boot_cpu(void)
/* Get a CPU thread count based on family */
switch(proc_gen) {
case proc_gen_p8:
- cpu_thread_count = 8;
+ cpu_threads_max = 8;
prlog(PR_INFO, "CPU: P8 generation processor"
- " (max %d threads/core)\n", cpu_thread_count);
+ " (max %d threads/core)\n", cpu_threads_max);
break;
case proc_gen_p9:
if (is_fused_core(pvr))
- cpu_thread_count = 8;
+ cpu_threads_max = 8;
else
- cpu_thread_count = 4;
+ cpu_threads_max = 4;
prlog(PR_INFO, "CPU: P9 generation processor"
- " (max %d threads/core)\n", cpu_thread_count);
+ " (max %d threads/core)\n", cpu_threads_max);
break;
case proc_gen_p10:
if (is_fused_core(pvr))
- cpu_thread_count = 8;
+ cpu_threads_max = 8;
else
- cpu_thread_count = 4;
+ cpu_threads_max = 4;
prlog(PR_INFO, "CPU: P10 generation processor"
- " (max %d threads/core)\n", cpu_thread_count);
+ " (max %d threads/core)\n", cpu_threads_max);
break;
default:
prerror("CPU: Unknown PVR, assuming 1 thread\n");
- cpu_thread_count = 1;
+ cpu_threads_max = 1;
}
if (proc_gen == proc_gen_p8) {
@@ -1192,7 +1193,8 @@ void init_cpu_max_pir(void)
/* Iterate all CPUs in the device-tree */
dt_for_each_child(cpus, cpu) {
- unsigned int pir, server_no;
+ unsigned int pir, server_no, threads;
+ const struct dt_property *p;
/* Skip cache nodes */
if (strcmp(dt_prop_get(cpu, "device_type"), "cpu"))
@@ -1205,8 +1207,27 @@ void init_cpu_max_pir(void)
*/
pir = dt_prop_get_u32_def(cpu, "ibm,pir", server_no);
- if (cpu_max_pir < pir + cpu_thread_count - 1)
- cpu_max_pir = pir + cpu_thread_count - 1;
+ p = dt_find_property(cpu, "ibm,ppc-interrupt-server#s");
+ if (!p)
+ continue;
+ threads = p->len / 4;
+ assert(threads > 0);
+ if (threads > cpu_threads_max) {
+ prlog(PR_WARNING, "CPU: Threads out of range for PIR 0x%04x"
+ " threads=%d max=%d\n",
+ pir, threads, cpu_threads_max);
+ threads = cpu_threads_max;
+ }
+ if (!cpu_thread_count) {
+ cpu_thread_count = threads;
+ } else {
+ /* Do not support asymmetric SMT topologies */
+ assert(cpu_thread_count == threads);
+ }
+
+
+ if (cpu_max_pir < pir + threads - 1)
+ cpu_max_pir = pir + threads - 1;
}
prlog(PR_DEBUG, "CPU: New max PIR set to 0x%x\n", cpu_max_pir);
@@ -1277,17 +1298,10 @@ void init_all_cpus(void)
prlog(PR_INFO, "CPU: CPU from DT PIR=0x%04x Server#=0x%x"
" State=%d\n", pir, server_no, state);
- /* Check max PIR */
- if (cpu_max_pir < (pir + cpu_thread_count - 1)) {
- prlog(PR_WARNING, "CPU: CPU potentially out of range"
- "PIR=0x%04x MAX=0x%04x !\n",
- pir, cpu_max_pir);
- continue;
- }
-
/* Setup thread 0 */
assert(pir <= cpu_max_pir);
t = pt0 = &cpu_stacks[pir].cpu;
+
if (t != boot_cpu) {
init_cpu_thread(t, state, pir);
/* Each cpu gets its own later in init_trace_buffers */
@@ -1322,12 +1336,6 @@ void init_all_cpus(void)
if (!p)
continue;
threads = p->len / 4;
- if (threads > cpu_thread_count) {
- prlog(PR_WARNING, "CPU: Threads out of range for PIR 0x%04x"
- " threads=%d max=%d\n",
- pir, threads, cpu_thread_count);
- threads = cpu_thread_count;
- }
for (thread = 1; thread < threads; thread++) {
prlog(PR_TRACE, "CPU: secondary thread %d found\n",
thread);
--
2.45.2
More information about the Skiboot
mailing list