[Qemu-devel] [RFC 1/4] numa, spapr: add thread-id in the possible_cpus list

[Laurent Vivier]

spapr_possible_cpu_arch_ids() counts only cores, and so
the number of available CPUs is the number of vCPU divided
by smp_threads.

... -smp 4,maxcpus=8,cores=2,threads=2,sockets=2 -numa node,cpus=0,cpus=1 \
                                                 -numa node,cpus=3,cpus=4 \
                                                 -numa node -numa node

This generates (info hotpluggable-cpus)

  node-id: 0 core-id: 0 thread-id: 0 [thread-id: 1]
  node-id: 0 core-id: 6 thread-id: 0 [thread-id: 1]
  node-id: 1 core-id: 2 thread-id: 0 [thread-id: 1]
  node-id: 1 core-id: 4 thread-id: 0 [thread-id: 1]

And this command line generates the following error:

  CPU(s) not present in any NUMA nodes: CPU 3 [core-id: 6]

That is wrong because CPU 3 [core-id: 6] is assigned to node-id 0
Moreover "cpus=4" is not valid, because it means core-id 8 but
maxcpus is 8.

With this patch we have now:

  node-id: 0 core-id: 0 thread-id: 0
  node-id: 0 core-id: 0 thread-id: 1
  node-id: 0 core-id: 1 thread-id: 0
  node-id: 1 core-id: 1 thread-id: 1
  node-id: 0 core-id: 2 thread-id: 1
  node-id: 1 core-id: 2 thread-id: 0
  node-id: 0 core-id: 3 thread-id: 1
  node-id: 0 core-id: 3 thread-id: 0

CPUs 0 (core-id: 0 thread-id: 0) and 1 (core-id: 0 thread-id: 1) are
correctly assigned to node-id 0, CPUs 3 (core-id: 1 thread-id: 1) and
 4 (core-id: 2 thread-id: 0) are correctly assigned to node-id 1.
All other CPUs are assigned to node-id 0 by default.

And the error message is also correct:

  CPU(s) not present in any NUMA nodes: CPU 2 [core-id: 1, thread-id: 0], \
                                        CPU 5 [core-id: 2, thread-id: 1], \
                                        CPU 6 [core-id: 3, thread-id: 0], \
                                        CPU 7 [core-id: 3, thread-id: 1]

Fixes: ec78f8114bc4 ("numa: use possible_cpus for not mapped CPUs check")
Cc: address@hidden

Before commit ec78f8114bc4, output was correct:

  CPU(s) not present in any NUMA nodes: 2 5 6 7

Signed-off-by: Laurent Vivier <address@hidden>
---
 hw/ppc/spapr.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 332cba89d425..7196ba09da34 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2404,15 +2404,13 @@ static void spapr_validate_node_memory(MachineState 
*machine, Error **errp)
 /* find cpu slot in machine->possible_cpus by core_id */
 static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
 {
-    int index = id / smp_threads;
-
-    if (index >= ms->possible_cpus->len) {
+    if (id >= ms->possible_cpus->len) {
         return NULL;
     }
     if (idx) {
-        *idx = index;
+        *idx = id;
     }
-    return &ms->possible_cpus->cpus[index];
+    return &ms->possible_cpus->cpus[id];
 }
 
 static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
@@ -2514,7 +2512,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
             error_report("This machine version does not support CPU hotplug");
             exit(1);
         }
-        boot_cores_nr = possible_cpus->len;
+        boot_cores_nr = possible_cpus->len / smp_threads;
     }
 
     if (smc->pre_2_10_has_unused_icps) {
@@ -2528,7 +2526,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
         }
     }
 
-    for (i = 0; i < possible_cpus->len; i++) {
+    for (i = 0; i < possible_cpus->len / smp_threads; i++) {
         int core_id = i * smp_threads;
 
         if (mc->has_hotpluggable_cpus) {
@@ -3795,21 +3793,16 @@ spapr_cpu_index_to_props(MachineState *machine, 
unsigned cpu_index)
 
 static int64_t spapr_get_default_cpu_node_id(const MachineState *ms, int idx)
 {
-    return idx / smp_cores % nb_numa_nodes;
+    return idx / (smp_cores * smp_threads) % nb_numa_nodes;
 }
 
 static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
 {
     int i;
     const char *core_type;
-    int spapr_max_cores = max_cpus / smp_threads;
-    MachineClass *mc = MACHINE_GET_CLASS(machine);
 
-    if (!mc->has_hotpluggable_cpus) {
-        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
-    }
     if (machine->possible_cpus) {
-        assert(machine->possible_cpus->len == spapr_max_cores);
+        assert(machine->possible_cpus->len == max_cpus);
         return machine->possible_cpus;
     }
 
@@ -3820,16 +3813,16 @@ static const CPUArchIdList 
*spapr_possible_cpu_arch_ids(MachineState *machine)
     }
 
     machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
-                             sizeof(CPUArchId) * spapr_max_cores);
-    machine->possible_cpus->len = spapr_max_cores;
+                             sizeof(CPUArchId) * max_cpus);
+    machine->possible_cpus->len = max_cpus;
     for (i = 0; i < machine->possible_cpus->len; i++) {
-        int core_id = i * smp_threads;
-
         machine->possible_cpus->cpus[i].type = core_type;
         machine->possible_cpus->cpus[i].vcpus_count = smp_threads;
-        machine->possible_cpus->cpus[i].arch_id = core_id;
+        machine->possible_cpus->cpus[i].arch_id = i;
         machine->possible_cpus->cpus[i].props.has_core_id = true;
-        machine->possible_cpus->cpus[i].props.core_id = core_id;
+        machine->possible_cpus->cpus[i].props.core_id = i / smp_threads;
+        machine->possible_cpus->cpus[i].props.has_thread_id = true;
+        machine->possible_cpus->cpus[i].props.thread_id = i % smp_threads;
     }
     return machine->possible_cpus;
 }
-- 
2.20.1