| Message ID | 1493816238-33120-7-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 03, 2017 at 02:57:00PM +0200, Igor Mammedov wrote: > it will allow switching from cpu_index to core based numa > mapping in follow up patches. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Have you considered adding this to TYPE_CPU instead of duplicating the same code on multiple architectures/boards? > --- > include/hw/ppc/spapr_cpu_core.h | 1 + > include/qom/cpu.h | 2 ++ > hw/ppc/spapr.c | 17 +++++++++++++++++ > hw/ppc/spapr_cpu_core.c | 11 ++++++++--- > 4 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > index 3c35665..93051e9 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -27,6 +27,7 @@ typedef struct sPAPRCPUCore { > > /*< public >*/ > void *threads; > + int node_id; > } sPAPRCPUCore; > > typedef struct sPAPRCPUCoreClass { > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 5d10359..55214ce 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -258,6 +258,8 @@ typedef void (*run_on_cpu_func)(CPUState *cpu, run_on_cpu_data data); > > struct qemu_work_item; > > +#define CPU_UNSET_NUMA_NODE_ID -1 > + > /** > * CPUState: > * @cpu_index: CPU index (informative). > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 33405a0..7f58ee4 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2824,9 +2824,11 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev); > Error *local_err = NULL; > CPUCore *cc = CPU_CORE(dev); > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model); > const char *type = object_get_typename(OBJECT(dev)); > CPUArchId *core_slot; > + int node_id; > int index; > > if (dev->hotplugged && !mc->has_hotpluggable_cpus) { > @@ -2861,6 +2863,21 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > goto out; > } > > + node_id = numa_get_node_for_cpu(cc->core_id); > + if (node_id == nb_numa_nodes) { > + /* by default CPUState::numa_node was 0 if it's not set via CLI > + * keep it this way for now but in future we probably should > + * refuse to start up with incomplete numa mapping */ > + node_id = 0; > + } > + if (sc->node_id == CPU_UNSET_NUMA_NODE_ID) { > + sc->node_id = node_id; > + } else if (sc->node_id != node_id) { > + error_setg(&local_err, "node-id %d must match numa node specified" > + "with -numa option for cpu-index %d", sc->node_id, cc->core_id); > + goto out; > + } > + > out: > g_free(base_core_type); > error_propagate(errp, local_err); > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 4389ef4..9de7a56 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -176,7 +176,6 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > const char *typename = object_class_get_name(scc->cpu_class); > size_t size = object_type_get_instance_size(typename); > Error *local_err = NULL; > - int core_node_id = numa_get_node_for_cpu(cc->core_id);; > void *obj; > int i, j; > > @@ -194,10 +193,10 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > /* Set NUMA node for the added CPUs */ > node_id = numa_get_node_for_cpu(cs->cpu_index); > - if (node_id != core_node_id) { > + if (node_id != sc->node_id) { > error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > " on CPU[core-id: %d, node-id: %d], node-id must be the same", > - node_id, cs->cpu_index, cc->core_id, core_node_id); > + node_id, cs->cpu_index, cc->core_id, sc->node_id); > goto err; > } > if (node_id < nb_numa_nodes) { > @@ -263,6 +262,11 @@ static const char *spapr_core_models[] = { > "POWER9_v1.0", > }; > > +static Property spapr_cpu_core_properties[] = { > + DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID), > + DEFINE_PROP_END_OF_LIST() > +}; > + > void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > @@ -270,6 +274,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > dc->realize = spapr_cpu_core_realize; > dc->unrealize = spapr_cpu_core_unrealizefn; > + dc->props = spapr_cpu_core_properties; > scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); > g_assert(scc->cpu_class); > } > -- > 2.7.4 >
On Wed, 3 May 2017 11:46:44 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Wed, May 03, 2017 at 02:57:00PM +0200, Igor Mammedov wrote: > > it will allow switching from cpu_index to core based numa > > mapping in follow up patches. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Have you considered adding this to TYPE_CPU instead of > duplicating the same code on multiple architectures/boards? it was in TYPE_CPU at RFC time, but it adds public node-id property to every CPU. So I've rewrote it they way it would affect only necessary CPUs, it might be possible to generalize node_id sanity checks at pre_plug time into a common wrapper, I can do it on top so it would be visible what it's generalized (or an extra patch on respin) > > > --- > > include/hw/ppc/spapr_cpu_core.h | 1 + > > include/qom/cpu.h | 2 ++ > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > hw/ppc/spapr_cpu_core.c | 11 ++++++++--- > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > > index 3c35665..93051e9 100644 > > --- a/include/hw/ppc/spapr_cpu_core.h > > +++ b/include/hw/ppc/spapr_cpu_core.h > > @@ -27,6 +27,7 @@ typedef struct sPAPRCPUCore { > > > > /*< public >*/ > > void *threads; > > + int node_id; > > } sPAPRCPUCore; > > > > typedef struct sPAPRCPUCoreClass { > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > index 5d10359..55214ce 100644 > > --- a/include/qom/cpu.h > > +++ b/include/qom/cpu.h > > @@ -258,6 +258,8 @@ typedef void (*run_on_cpu_func)(CPUState *cpu, run_on_cpu_data data); > > > > struct qemu_work_item; > > > > +#define CPU_UNSET_NUMA_NODE_ID -1 > > + > > /** > > * CPUState: > > * @cpu_index: CPU index (informative). > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 33405a0..7f58ee4 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2824,9 +2824,11 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev); > > Error *local_err = NULL; > > CPUCore *cc = CPU_CORE(dev); > > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > > char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model); > > const char *type = object_get_typename(OBJECT(dev)); > > CPUArchId *core_slot; > > + int node_id; > > int index; > > > > if (dev->hotplugged && !mc->has_hotpluggable_cpus) { > > @@ -2861,6 +2863,21 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > goto out; > > } > > > > + node_id = numa_get_node_for_cpu(cc->core_id); > > + if (node_id == nb_numa_nodes) { > > + /* by default CPUState::numa_node was 0 if it's not set via CLI > > + * keep it this way for now but in future we probably should > > + * refuse to start up with incomplete numa mapping */ > > + node_id = 0; > > + } > > + if (sc->node_id == CPU_UNSET_NUMA_NODE_ID) { > > + sc->node_id = node_id; > > + } else if (sc->node_id != node_id) { > > + error_setg(&local_err, "node-id %d must match numa node specified" > > + "with -numa option for cpu-index %d", sc->node_id, cc->core_id); > > + goto out; > > + } > > + > > out: > > g_free(base_core_type); > > error_propagate(errp, local_err); > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 4389ef4..9de7a56 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -176,7 +176,6 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > const char *typename = object_class_get_name(scc->cpu_class); > > size_t size = object_type_get_instance_size(typename); > > Error *local_err = NULL; > > - int core_node_id = numa_get_node_for_cpu(cc->core_id);; > > void *obj; > > int i, j; > > > > @@ -194,10 +193,10 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > > > /* Set NUMA node for the added CPUs */ > > node_id = numa_get_node_for_cpu(cs->cpu_index); > > - if (node_id != core_node_id) { > > + if (node_id != sc->node_id) { > > error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > > " on CPU[core-id: %d, node-id: %d], node-id must be the same", > > - node_id, cs->cpu_index, cc->core_id, core_node_id); > > + node_id, cs->cpu_index, cc->core_id, sc->node_id); > > goto err; > > } > > if (node_id < nb_numa_nodes) { > > @@ -263,6 +262,11 @@ static const char *spapr_core_models[] = { > > "POWER9_v1.0", > > }; > > > > +static Property spapr_cpu_core_properties[] = { > > + DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID), > > + DEFINE_PROP_END_OF_LIST() > > +}; > > + > > void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > { > > DeviceClass *dc = DEVICE_CLASS(oc); > > @@ -270,6 +274,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > > > dc->realize = spapr_cpu_core_realize; > > dc->unrealize = spapr_cpu_core_unrealizefn; > > + dc->props = spapr_cpu_core_properties; > > scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); > > g_assert(scc->cpu_class); > > } > > -- > > 2.7.4 > > >
On Wed, May 03, 2017 at 06:12:17PM +0200, Igor Mammedov wrote: > On Wed, 3 May 2017 11:46:44 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Wed, May 03, 2017 at 02:57:00PM +0200, Igor Mammedov wrote: > > > it will allow switching from cpu_index to core based numa > > > mapping in follow up patches. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > > > Have you considered adding this to TYPE_CPU instead of > > duplicating the same code on multiple architectures/boards? > it was in TYPE_CPU at RFC time, but it adds public node-id > property to every CPU. > So I've rewrote it they way it would affect only necessary CPUs, > > it might be possible to generalize node_id sanity checks at > pre_plug time into a common wrapper, I can do it on top so > it would be visible what it's generalized (or an extra patch on > respin) Note that the spapr cpu core type here is *not* a descendent of TYPE_CPU (the threads under it are). > > > > > > --- > > > include/hw/ppc/spapr_cpu_core.h | 1 + > > > include/qom/cpu.h | 2 ++ > > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > > hw/ppc/spapr_cpu_core.c | 11 ++++++++--- > > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > > > index 3c35665..93051e9 100644 > > > --- a/include/hw/ppc/spapr_cpu_core.h > > > +++ b/include/hw/ppc/spapr_cpu_core.h > > > @@ -27,6 +27,7 @@ typedef struct sPAPRCPUCore { > > > > > > /*< public >*/ > > > void *threads; > > > + int node_id; > > > } sPAPRCPUCore; > > > > > > typedef struct sPAPRCPUCoreClass { > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > > index 5d10359..55214ce 100644 > > > --- a/include/qom/cpu.h > > > +++ b/include/qom/cpu.h > > > @@ -258,6 +258,8 @@ typedef void (*run_on_cpu_func)(CPUState *cpu, run_on_cpu_data data); > > > > > > struct qemu_work_item; > > > > > > +#define CPU_UNSET_NUMA_NODE_ID -1 > > > + > > > /** > > > * CPUState: > > > * @cpu_index: CPU index (informative). > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 33405a0..7f58ee4 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -2824,9 +2824,11 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > > MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev); > > > Error *local_err = NULL; > > > CPUCore *cc = CPU_CORE(dev); > > > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > > > char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model); > > > const char *type = object_get_typename(OBJECT(dev)); > > > CPUArchId *core_slot; > > > + int node_id; > > > int index; > > > > > > if (dev->hotplugged && !mc->has_hotpluggable_cpus) { > > > @@ -2861,6 +2863,21 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > > goto out; > > > } > > > > > > + node_id = numa_get_node_for_cpu(cc->core_id); > > > + if (node_id == nb_numa_nodes) { > > > + /* by default CPUState::numa_node was 0 if it's not set via CLI > > > + * keep it this way for now but in future we probably should > > > + * refuse to start up with incomplete numa mapping */ > > > + node_id = 0; > > > + } > > > + if (sc->node_id == CPU_UNSET_NUMA_NODE_ID) { > > > + sc->node_id = node_id; > > > + } else if (sc->node_id != node_id) { > > > + error_setg(&local_err, "node-id %d must match numa node specified" > > > + "with -numa option for cpu-index %d", sc->node_id, cc->core_id); > > > + goto out; > > > + } > > > + > > > out: > > > g_free(base_core_type); > > > error_propagate(errp, local_err); > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > > index 4389ef4..9de7a56 100644 > > > --- a/hw/ppc/spapr_cpu_core.c > > > +++ b/hw/ppc/spapr_cpu_core.c > > > @@ -176,7 +176,6 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > > const char *typename = object_class_get_name(scc->cpu_class); > > > size_t size = object_type_get_instance_size(typename); > > > Error *local_err = NULL; > > > - int core_node_id = numa_get_node_for_cpu(cc->core_id);; > > > void *obj; > > > int i, j; > > > > > > @@ -194,10 +193,10 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > > > > > /* Set NUMA node for the added CPUs */ > > > node_id = numa_get_node_for_cpu(cs->cpu_index); > > > - if (node_id != core_node_id) { > > > + if (node_id != sc->node_id) { > > > error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > > > " on CPU[core-id: %d, node-id: %d], node-id must be the same", > > > - node_id, cs->cpu_index, cc->core_id, core_node_id); > > > + node_id, cs->cpu_index, cc->core_id, sc->node_id); > > > goto err; > > > } > > > if (node_id < nb_numa_nodes) { > > > @@ -263,6 +262,11 @@ static const char *spapr_core_models[] = { > > > "POWER9_v1.0", > > > }; > > > > > > +static Property spapr_cpu_core_properties[] = { > > > + DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID), > > > + DEFINE_PROP_END_OF_LIST() > > > +}; > > > + > > > void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > > { > > > DeviceClass *dc = DEVICE_CLASS(oc); > > > @@ -270,6 +274,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > > > > > dc->realize = spapr_cpu_core_realize; > > > dc->unrealize = spapr_cpu_core_unrealizefn; > > > + dc->props = spapr_cpu_core_properties; > > > scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); > > > g_assert(scc->cpu_class); > > > } > > >
On Fri, 5 May 2017 02:49:52 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Wed, May 03, 2017 at 06:12:17PM +0200, Igor Mammedov wrote: > > On Wed, 3 May 2017 11:46:44 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > On Wed, May 03, 2017 at 02:57:00PM +0200, Igor Mammedov wrote: > > > > it will allow switching from cpu_index to core based numa > > > > mapping in follow up patches. > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > > > > > Have you considered adding this to TYPE_CPU instead of > > > duplicating the same code on multiple architectures/boards? > > it was in TYPE_CPU at RFC time, but it adds public node-id > > property to every CPU. > > So I've rewrote it they way it would affect only necessary CPUs, > > > > it might be possible to generalize node_id sanity checks at > > pre_plug time into a common wrapper, I can do it on top so > > it would be visible what it's generalized (or an extra patch on > > respin) > > Note that the spapr cpu core type here is *not* a descendent of > TYPE_CPU (the threads under it are). yep, what I was saying is that numa checks in pre_plug callbacks are very similar and it might be possible to replaces them with common function. > > > > > > > > > --- > > > > include/hw/ppc/spapr_cpu_core.h | 1 + > > > > include/qom/cpu.h | 2 ++ > > > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > > > hw/ppc/spapr_cpu_core.c | 11 ++++++++--- > > > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > > > > index 3c35665..93051e9 100644 > > > > --- a/include/hw/ppc/spapr_cpu_core.h > > > > +++ b/include/hw/ppc/spapr_cpu_core.h > > > > @@ -27,6 +27,7 @@ typedef struct sPAPRCPUCore { > > > > > > > > /*< public >*/ > > > > void *threads; > > > > + int node_id; > > > > } sPAPRCPUCore; > > > > > > > > typedef struct sPAPRCPUCoreClass { > > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > > > index 5d10359..55214ce 100644 > > > > --- a/include/qom/cpu.h > > > > +++ b/include/qom/cpu.h > > > > @@ -258,6 +258,8 @@ typedef void (*run_on_cpu_func)(CPUState *cpu, run_on_cpu_data data); > > > > > > > > struct qemu_work_item; > > > > > > > > +#define CPU_UNSET_NUMA_NODE_ID -1 > > > > + > > > > /** > > > > * CPUState: > > > > * @cpu_index: CPU index (informative). > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > > index 33405a0..7f58ee4 100644 > > > > --- a/hw/ppc/spapr.c > > > > +++ b/hw/ppc/spapr.c > > > > @@ -2824,9 +2824,11 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > > > MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev); > > > > Error *local_err = NULL; > > > > CPUCore *cc = CPU_CORE(dev); > > > > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > > > > char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model); > > > > const char *type = object_get_typename(OBJECT(dev)); > > > > CPUArchId *core_slot; > > > > + int node_id; > > > > int index; > > > > > > > > if (dev->hotplugged && !mc->has_hotpluggable_cpus) { > > > > @@ -2861,6 +2863,21 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > > > goto out; > > > > } > > > > > > > > + node_id = numa_get_node_for_cpu(cc->core_id); > > > > + if (node_id == nb_numa_nodes) { > > > > + /* by default CPUState::numa_node was 0 if it's not set via CLI > > > > + * keep it this way for now but in future we probably should > > > > + * refuse to start up with incomplete numa mapping */ > > > > + node_id = 0; > > > > + } > > > > + if (sc->node_id == CPU_UNSET_NUMA_NODE_ID) { > > > > + sc->node_id = node_id; > > > > + } else if (sc->node_id != node_id) { > > > > + error_setg(&local_err, "node-id %d must match numa node specified" > > > > + "with -numa option for cpu-index %d", sc->node_id, cc->core_id); > > > > + goto out; > > > > + } > > > > + > > > > out: > > > > g_free(base_core_type); > > > > error_propagate(errp, local_err); > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > > > index 4389ef4..9de7a56 100644 > > > > --- a/hw/ppc/spapr_cpu_core.c > > > > +++ b/hw/ppc/spapr_cpu_core.c > > > > @@ -176,7 +176,6 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > > > const char *typename = object_class_get_name(scc->cpu_class); > > > > size_t size = object_type_get_instance_size(typename); > > > > Error *local_err = NULL; > > > > - int core_node_id = numa_get_node_for_cpu(cc->core_id);; > > > > void *obj; > > > > int i, j; > > > > > > > > @@ -194,10 +193,10 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > > > > > > > /* Set NUMA node for the added CPUs */ > > > > node_id = numa_get_node_for_cpu(cs->cpu_index); > > > > - if (node_id != core_node_id) { > > > > + if (node_id != sc->node_id) { > > > > error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > > > > " on CPU[core-id: %d, node-id: %d], node-id must be the same", > > > > - node_id, cs->cpu_index, cc->core_id, core_node_id); > > > > + node_id, cs->cpu_index, cc->core_id, sc->node_id); > > > > goto err; > > > > } > > > > if (node_id < nb_numa_nodes) { > > > > @@ -263,6 +262,11 @@ static const char *spapr_core_models[] = { > > > > "POWER9_v1.0", > > > > }; > > > > > > > > +static Property spapr_cpu_core_properties[] = { > > > > + DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID), > > > > + DEFINE_PROP_END_OF_LIST() > > > > +}; > > > > + > > > > void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > > > { > > > > DeviceClass *dc = DEVICE_CLASS(oc); > > > > @@ -270,6 +274,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > > > > > > > > dc->realize = spapr_cpu_core_realize; > > > > dc->unrealize = spapr_cpu_core_unrealizefn; > > > > + dc->props = spapr_cpu_core_properties; > > > > scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); > > > > g_assert(scc->cpu_class); > > > > } > > > > > >
diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h index 3c35665..93051e9 100644 --- a/include/hw/ppc/spapr_cpu_core.h +++ b/include/hw/ppc/spapr_cpu_core.h @@ -27,6 +27,7 @@ typedef struct sPAPRCPUCore { /*< public >*/ void *threads; + int node_id; } sPAPRCPUCore; typedef struct sPAPRCPUCoreClass { diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 5d10359..55214ce 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -258,6 +258,8 @@ typedef void (*run_on_cpu_func)(CPUState *cpu, run_on_cpu_data data); struct qemu_work_item; +#define CPU_UNSET_NUMA_NODE_ID -1 + /** * CPUState: * @cpu_index: CPU index (informative). diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 33405a0..7f58ee4 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2824,9 +2824,11 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev); Error *local_err = NULL; CPUCore *cc = CPU_CORE(dev); + sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model); const char *type = object_get_typename(OBJECT(dev)); CPUArchId *core_slot; + int node_id; int index; if (dev->hotplugged && !mc->has_hotpluggable_cpus) { @@ -2861,6 +2863,21 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, goto out; } + node_id = numa_get_node_for_cpu(cc->core_id); + if (node_id == nb_numa_nodes) { + /* by default CPUState::numa_node was 0 if it's not set via CLI + * keep it this way for now but in future we probably should + * refuse to start up with incomplete numa mapping */ + node_id = 0; + } + if (sc->node_id == CPU_UNSET_NUMA_NODE_ID) { + sc->node_id = node_id; + } else if (sc->node_id != node_id) { + error_setg(&local_err, "node-id %d must match numa node specified" + "with -numa option for cpu-index %d", sc->node_id, cc->core_id); + goto out; + } + out: g_free(base_core_type); error_propagate(errp, local_err); diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 4389ef4..9de7a56 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -176,7 +176,6 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) const char *typename = object_class_get_name(scc->cpu_class); size_t size = object_type_get_instance_size(typename); Error *local_err = NULL; - int core_node_id = numa_get_node_for_cpu(cc->core_id);; void *obj; int i, j; @@ -194,10 +193,10 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) /* Set NUMA node for the added CPUs */ node_id = numa_get_node_for_cpu(cs->cpu_index); - if (node_id != core_node_id) { + if (node_id != sc->node_id) { error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" " on CPU[core-id: %d, node-id: %d], node-id must be the same", - node_id, cs->cpu_index, cc->core_id, core_node_id); + node_id, cs->cpu_index, cc->core_id, sc->node_id); goto err; } if (node_id < nb_numa_nodes) { @@ -263,6 +262,11 @@ static const char *spapr_core_models[] = { "POWER9_v1.0", }; +static Property spapr_cpu_core_properties[] = { + DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, CPU_UNSET_NUMA_NODE_ID), + DEFINE_PROP_END_OF_LIST() +}; + void spapr_cpu_core_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); @@ -270,6 +274,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) dc->realize = spapr_cpu_core_realize; dc->unrealize = spapr_cpu_core_unrealizefn; + dc->props = spapr_cpu_core_properties; scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); g_assert(scc->cpu_class); }