On Fri, Mar 31, 2017 at 10:21 AM, David Gibson <da...@gibson.dropbear.id.au>
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 <da...@gibson.dropbear.id.au>
> ---
> 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.
