Re: [Qemu-ppc] [Qemu-devel] [PATCH for-2.9] pseries: Enforce homogeneous threads-per-core

[Bharata B Rao]

On Fri, Mar 31, 2017 at 10:21 AM, David Gibson <address@hidden> wrote:

For reasons that may be useful in future, CPU core objects, as used on the
pseries machine type have their own nr-threads property, potentially
allowing cores with different numbers of threads in the same system.

I remember we retained this flexibility to support heterogenous systems in future ? I think we can go with this enforcement now and relax it later if we ever reach there.
 

If the user/management uses the values specified in query-hotpluggable-cpus
as they're expected to do, this will never matter in pratice.  But that's
not actually enforced - it's possible to manually specify a core with
a different number of threads from that in -smp.  That will confuse the
platform - most immediately, this can be used to create a CPU thread with
index above max_cpus which leads to an assertion failure in
spapr_cpu_core_realize().

For now, enforce that all cores must have the same, standard, number of
threads.  While we're at it, also enforce that the core ids are correctly
aligned based on that number of threads.

Signed-off-by: David Gibson <address@hidden>
---
 hw/ppc/spapr_cpu_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 6883f09..935ee62 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -167,6 +167,18 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
     void *obj;
     int i, j;

+    if (cc->nr_threads != smp_threads) {
+        error_setg(errp,
+                   "Invalid nr-threads=%d of CPU[core-id: %d], must be %d",
+                   cc->nr_threads, cc->core_id, smp_threads);
+        return;
+    }

The above check should move to pre_plug handler.
 

+    if ((cc->core_id % smp_threads) != 0) {
+        error_setg(errp, "Invalid CPU core-id=%d, must be a multiple of %d",
+                   cc->core_id, smp_threads);
+        return;
+    }

Not sure when you will hit this as the same check is already  present in pre_plug handler.

Regards,

Bharata.