* [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug
@ 2016-02-22 5:01 Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState Bharata B Rao
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Hi,
This is an attempt to implement David Gibson's RFC that was posted at
https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html
I am not sure if I have followed all the aspects of the RFC fully, but we
can make changes going forward.
An example cpu-package implementation is done for sPAPR in this patchset.
Hot removal is not yet done in this patchset.
For the command line,
-smp 8,sockets=1,cores=1,threads=8,maxcpus=16 -numa node,nodeid=0,cpus=0-7 -numa node,nodeid=1,cpus=8-15
the HMP query looks like this:
(qemu) info cpu-packages
CPU Package: ""
type: "spapr-cpu-package"
qom_path: "/machine/cpu-package[0]"
realized: true
nr_cpus: 8
CPU: 0
Type: "host-powerpc64-cpu"
Arch ID: 0
Thread: 0
Core: 0
Socket: 0
Node: 0
CPU: 1
Type: "host-powerpc64-cpu"
Arch ID: 1
Thread: 1
Core: 0
Socket: 0
Node: 0
CPU: 2
Type: "host-powerpc64-cpu"
Arch ID: 2
Thread: 2
Core: 0
Socket: 0
Node: 0
CPU: 3
Type: "host-powerpc64-cpu"
Arch ID: 3
Thread: 3
Core: 0
Socket: 0
Node: 0
CPU: 4
Type: "host-powerpc64-cpu"
Arch ID: 4
Thread: 4
Core: 0
Socket: 0
Node: 0
CPU: 5
Type: "host-powerpc64-cpu"
Arch ID: 5
Thread: 5
Core: 0
Socket: 0
Node: 0
CPU: 6
Type: "host-powerpc64-cpu"
Arch ID: 6
Thread: 6
Core: 0
Socket: 0
Node: 0
CPU: 7
Type: "host-powerpc64-cpu"
Arch ID: 7
Thread: 7
Core: 0
Socket: 0
Node: 0
CPU Package: ""
type: "spapr-cpu-package"
qom_path: "/machine/cpu-package[1]"
realized: false
nr_cpus: 8
As can be seen from above, all the cores upto max_cpus are created upfront
here and hot plug is done in the following manner:
(qemu) qom-set /machine/cpu-package[1] realized true
This will result in the 2nd cpu-package consisting of a core with 8 threads
to become available.
I am not fully sure if the QMP emumeration here works for all archs, but
just wanted to share what I currently.
Bharata B Rao (8):
cpu: Store CPU typename in MachineState
cpu: Don't realize CPU from cpu_generic_init()
cpu: CPU package abstract device
spapr: Introduce CPU core device
spapr: Convert boot CPUs into CPU core device initialization
spapr: CPU hotplug support
qmp: Implement query cpu-packages
hmp: Implement 'info cpu-slots'
hmp-commands-info.hx | 14 +++
hmp.c | 50 ++++++++
hmp.h | 1 +
hw/cpu/Makefile.objs | 1 +
hw/cpu/package.c | 85 +++++++++++++
hw/ppc/Makefile.objs | 1 +
hw/ppc/spapr.c | 246 +++++++++++++++++++++++++++++++++++--
hw/ppc/spapr_cpu_package.c | 50 ++++++++
hw/ppc/spapr_events.c | 3 +
hw/ppc/spapr_rtas.c | 24 ++++
include/hw/boards.h | 2 +
include/hw/cpu/package.h | 27 ++++
include/hw/ppc/spapr.h | 1 +
include/hw/ppc/spapr_cpu_package.h | 27 ++++
qapi-schema.json | 48 ++++++++
qom/cpu.c | 6 -
target-arm/helper.c | 16 ++-
target-cris/cpu.c | 16 ++-
target-lm32/helper.c | 16 ++-
target-moxie/cpu.c | 16 ++-
target-openrisc/cpu.c | 16 ++-
target-ppc/translate_init.c | 16 ++-
target-sh4/cpu.c | 16 ++-
target-tricore/helper.c | 16 ++-
target-unicore32/helper.c | 16 ++-
25 files changed, 707 insertions(+), 23 deletions(-)
create mode 100644 hw/cpu/package.c
create mode 100644 hw/ppc/spapr_cpu_package.c
create mode 100644 include/hw/cpu/package.h
create mode 100644 include/hw/ppc/spapr_cpu_package.h
--
2.1.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 8:04 ` David Gibson
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 2/8] cpu: Don't realize CPU from cpu_generic_init() Bharata B Rao
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Storing CPU typename in MachineState lets us to create CPU threads
for all architectures in uniform manner from arch-neutral code.
TODO: Touching only sPAPR target for now
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 2 ++
include/hw/boards.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5bd8fd3..3892a99 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1823,6 +1823,8 @@ static void ppc_spapr_init(MachineState *machine)
if (machine->cpu_model == NULL) {
machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
}
+ machine->cpu_type = TYPE_POWERPC_CPU;
+
for (i = 0; i < smp_cpus; i++) {
cpu = cpu_ppc_init(machine->cpu_model);
if (cpu == NULL) {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..cf95d10 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -137,6 +137,7 @@ struct MachineState {
char *kernel_cmdline;
char *initrd_filename;
const char *cpu_model;
+ const char *cpu_type;
AccelState *accelerator;
};
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 2/8] cpu: Don't realize CPU from cpu_generic_init()
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 3/8] cpu: CPU package abstract device Bharata B Rao
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Don't do CPU realization from cpu_generic_init(). With this
cpu_generic_init() will be used to just create CPU threads and they
should be realized separately from realizefn call.
Convert the existing callers to do explicit realization.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
qom/cpu.c | 6 ------
target-arm/helper.c | 16 +++++++++++++++-
target-cris/cpu.c | 16 +++++++++++++++-
target-lm32/helper.c | 16 +++++++++++++++-
target-moxie/cpu.c | 16 +++++++++++++++-
target-openrisc/cpu.c | 16 +++++++++++++++-
target-ppc/translate_init.c | 16 +++++++++++++++-
target-sh4/cpu.c | 16 +++++++++++++++-
target-tricore/helper.c | 16 +++++++++++++++-
target-unicore32/helper.c | 16 +++++++++++++++-
10 files changed, 135 insertions(+), 15 deletions(-)
diff --git a/qom/cpu.c b/qom/cpu.c
index 38dc713..c0211fa 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -64,13 +64,7 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
featurestr = strtok(NULL, ",");
cc->parse_features(cpu, featurestr, &err);
g_free(str);
- if (err != NULL) {
- goto out;
- }
-
- object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-out:
if (err != NULL) {
error_report_err(err);
object_unref(OBJECT(cpu));
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5ea507f..d76c55c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4564,7 +4564,21 @@ void register_cp_regs_for_features(ARMCPU *cpu)
ARMCPU *cpu_arm_init(const char *cpu_model)
{
- return ARM_CPU(cpu_generic_init(TYPE_ARM_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_ARM_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return ARM_CPU(cpu);
+ }
}
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index b2c8624..82f0ce5 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -90,7 +90,21 @@ static ObjectClass *cris_cpu_class_by_name(const char *cpu_model)
CRISCPU *cpu_cris_init(const char *cpu_model)
{
- return CRIS_CPU(cpu_generic_init(TYPE_CRIS_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_CRIS_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return CRIS_CPU(cpu);
+ }
}
/* Sort alphabetically by VR. */
diff --git a/target-lm32/helper.c b/target-lm32/helper.c
index 655248f..4080496 100644
--- a/target-lm32/helper.c
+++ b/target-lm32/helper.c
@@ -220,7 +220,21 @@ bool lm32_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
LM32CPU *cpu_lm32_init(const char *cpu_model)
{
- return LM32_CPU(cpu_generic_init(TYPE_LM32_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_LM32_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return LM32_CPU(cpu);
+ }
}
/* Some soc ignores the MSB on the address bus. Thus creating a shadow memory
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index b33c2b3..95fc3d0 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -153,7 +153,21 @@ static const MoxieCPUInfo moxie_cpus[] = {
MoxieCPU *cpu_moxie_init(const char *cpu_model)
{
- return MOXIE_CPU(cpu_generic_init(TYPE_MOXIE_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_MOXIE_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return MOXIE_CPU(cpu);
+ }
}
static void cpu_register(const MoxieCPUInfo *info)
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index cafc07f..e82bcfc 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -223,7 +223,21 @@ static void openrisc_cpu_register_types(void)
OpenRISCCPU *cpu_openrisc_init(const char *cpu_model)
{
- return OPENRISC_CPU(cpu_generic_init(TYPE_OPENRISC_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_OPENRISC_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return OPENRISC_CPU(cpu);
+ }
}
/* Sort alphabetically by type name, except for "any". */
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index cdd18ac..a577bec 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9458,7 +9458,21 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name)
PowerPCCPU *cpu_ppc_init(const char *cpu_model)
{
- return POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_POWERPC_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return POWERPC_CPU(cpu);
+ }
}
/* Sort by PVR, ordering special case "host" last. */
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index 8621d70..db02ac2 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -156,7 +156,21 @@ static ObjectClass *superh_cpu_class_by_name(const char *cpu_model)
SuperHCPU *cpu_sh4_init(const char *cpu_model)
{
- return SUPERH_CPU(cpu_generic_init(TYPE_SUPERH_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_SUPERH_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return SUPERH_CPU(cpu);
+ }
}
static void sh7750r_cpu_initfn(Object *obj)
diff --git a/target-tricore/helper.c b/target-tricore/helper.c
index a8fd418..b9aa497 100644
--- a/target-tricore/helper.c
+++ b/target-tricore/helper.c
@@ -79,7 +79,21 @@ int cpu_tricore_handle_mmu_fault(CPUState *cs, target_ulong address,
TriCoreCPU *cpu_tricore_init(const char *cpu_model)
{
- return TRICORE_CPU(cpu_generic_init(TYPE_TRICORE_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_TRICORE_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return TRICORE_CPU(cpu);
+ }
}
static void tricore_cpu_list_entry(gpointer data, gpointer user_data)
diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
index 21f5f35..f0209b7 100644
--- a/target-unicore32/helper.c
+++ b/target-unicore32/helper.c
@@ -28,7 +28,21 @@
UniCore32CPU *uc32_cpu_init(const char *cpu_model)
{
- return UNICORE32_CPU(cpu_generic_init(TYPE_UNICORE32_CPU, cpu_model));
+ CPUState *cpu = cpu_generic_init(TYPE_UNICORE32_CPU, cpu_model);
+ Error *err = NULL;
+
+ if (!cpu) {
+ return NULL;
+ }
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_report_err(err);
+ object_unref(OBJECT(cpu));
+ return NULL;
+ } else {
+ return UNICORE32_CPU(cpu);
+ }
}
uint32_t HELPER(clo)(uint32_t x)
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 3/8] cpu: CPU package abstract device
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 2/8] cpu: Don't realize CPU from cpu_generic_init() Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device Bharata B Rao
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
A minimal abstract device that target machines can create sub-types of
to define their own cpu-package devices. Provides a realize routine that
walks the child objects and realizes them iteratively. Hence cpu-package
interface expects the target implementations to have a hierarchical
setup for their CPU objects.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/cpu/Makefile.objs | 1 +
hw/cpu/package.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/cpu/package.h | 27 ++++++++++++++++++++
3 files changed, 94 insertions(+)
create mode 100644 hw/cpu/package.c
create mode 100644 include/hw/cpu/package.h
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index 0954a18..f540826 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -2,4 +2,5 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
obj-$(CONFIG_REALVIEW) += realview_mpcore.o
obj-$(CONFIG_A9MPCORE) += a9mpcore.o
obj-$(CONFIG_A15MPCORE) += a15mpcore.o
+obj-y += package.o
diff --git a/hw/cpu/package.c b/hw/cpu/package.c
new file mode 100644
index 0000000..259dbfa
--- /dev/null
+++ b/hw/cpu/package.c
@@ -0,0 +1,66 @@
+/*
+ * CPU package device
+ *
+ * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "hw/cpu/package.h"
+#include "qom/object_interfaces.h"
+
+Object *cpu_package_create_object(const char *typename, uint32_t index,
+ Error **errp)
+{
+ char *id;
+ Object *obj;
+
+ id = g_strdup_printf("" TYPE_CPU_PACKAGE "[%"PRIu32"]", index);
+ obj = object_new(typename);
+ object_property_add_child(qdev_get_machine(), id, obj, errp);
+ g_free(id);
+
+ if (*errp) {
+ return NULL;
+ } else {
+ return obj;
+ }
+}
+
+static int cpu_package_realize_child(Object *child, void *opaque)
+{
+ Error **errp = opaque;
+
+ object_property_set_bool(child, true, "realized", errp);
+ if (*errp) {
+ return 1;
+ }
+ return 0;
+}
+
+static void cpu_package_realize(DeviceState *dev, Error **errp)
+{
+ object_child_foreach(OBJECT(dev), cpu_package_realize_child, errp);
+}
+
+static void cpu_package_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = cpu_package_realize;
+}
+
+static const TypeInfo cpu_package_info = {
+ .name = TYPE_CPU_PACKAGE,
+ .parent = TYPE_DEVICE,
+ .abstract = true,
+ .instance_size = sizeof(CPUPackage),
+ .class_init = cpu_package_class_init,
+};
+
+static void cpu_package_register_types(void)
+{
+ type_register_static(&cpu_package_info);
+}
+
+type_init(cpu_package_register_types)
diff --git a/include/hw/cpu/package.h b/include/hw/cpu/package.h
new file mode 100644
index 0000000..0579a42
--- /dev/null
+++ b/include/hw/cpu/package.h
@@ -0,0 +1,27 @@
+/*
+ * CPU package device
+ *
+ * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef HW_CPU_PACKAGE_H
+#define HW_CPU_PACKAGE_H
+
+#include "hw/qdev.h"
+
+#define TYPE_CPU_PACKAGE "cpu-package"
+#define CPU_PACKAGE(obj) \
+ OBJECT_CHECK(CPUPackage, (obj), TYPE_CPU_PACKAGE)
+
+typedef struct CPUPackage {
+ /* private */
+ DeviceState parent_obj;
+
+ /* public */
+} CPUPackage;
+
+Object *cpu_package_create_object(const char *typename, uint32_t index,
+ Error **errp);
+#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (2 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 3/8] cpu: CPU package abstract device Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 6:44 ` Andreas Färber
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 5/8] spapr: Convert boot CPUs into CPU core device initialization Bharata B Rao
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
performed in the granularity of CPU core device by setting the "realized"
property of this device to "true". When hotplugged, CPU core creates CPU
thread devices.
TODO: Right now allows for only homogeneous configurations as we depend
on global smp_threads and machine->cpu_model.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/ppc/Makefile.objs | 1 +
hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
3 files changed, 77 insertions(+)
create mode 100644 hw/ppc/spapr_cpu_package.c
create mode 100644 include/hw/ppc/spapr_cpu_package.h
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index c1ffc77..3000982 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
+obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
obj-y += spapr_pci_vfio.o
endif
diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
new file mode 100644
index 0000000..3120a16
--- /dev/null
+++ b/hw/ppc/spapr_cpu_package.c
@@ -0,0 +1,50 @@
+/*
+ * sPAPR CPU package device, acts as container of CPU thread devices.
+ *
+ * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "hw/cpu/package.h"
+#include "hw/ppc/spapr_cpu_package.h"
+#include "hw/boards.h"
+#include <sysemu/cpus.h>
+#include "qemu/error-report.h"
+
+static void spapr_cpu_package_instance_init(Object *obj)
+{
+ int i;
+ CPUState *cpu;
+ MachineState *machine = MACHINE(qdev_get_machine());
+ sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
+
+ /* Create as many CPU threads as specified in the topology */
+ for (i = 0; i < smp_threads; i++) {
+ cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
+ if (!cpu) {
+ error_setg(&error_fatal, "Unable to find CPU definition: %s\n",
+ machine->cpu_model);
+ }
+ object_property_add_child(obj, "thread[*]", OBJECT(cpu), &error_fatal);
+ object_unref(OBJECT(cpu));
+ DEVICE(OBJECT(cpu))->hotplugged = true;
+ if (!i) {
+ package->thread0 = POWERPC_CPU(cpu);
+ }
+ }
+}
+
+static const TypeInfo spapr_cpu_package_type_info = {
+ .name = TYPE_SPAPR_CPU_PACKAGE,
+ .parent = TYPE_CPU_PACKAGE,
+ .instance_init = spapr_cpu_package_instance_init,
+ .instance_size = sizeof(sPAPRCPUPackage),
+};
+
+static void spapr_cpu_package_register_types(void)
+{
+ type_register_static(&spapr_cpu_package_type_info);
+}
+
+type_init(spapr_cpu_package_register_types)
diff --git a/include/hw/ppc/spapr_cpu_package.h b/include/hw/ppc/spapr_cpu_package.h
new file mode 100644
index 0000000..547dbc1
--- /dev/null
+++ b/include/hw/ppc/spapr_cpu_package.h
@@ -0,0 +1,26 @@
+/*
+ * sPAPR CPU package device.
+ *
+ * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef HW_SPAPR_CPU_PACKAGE_H
+#define HW_SPAPR_CPU_PACKAGE_H
+
+#include "hw/qdev.h"
+
+#define TYPE_SPAPR_CPU_PACKAGE "spapr-cpu-package"
+#define SPAPR_CPU_PACKAGE(obj) \
+ OBJECT_CHECK(sPAPRCPUPackage, (obj), TYPE_SPAPR_CPU_PACKAGE)
+
+typedef struct sPAPRCPUPackage {
+ /*< private >*/
+ DeviceState parent_obj;
+
+ /*< public >*/
+ PowerPCCPU *thread0;
+} sPAPRCPUPackage;
+
+#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 5/8] spapr: Convert boot CPUs into CPU core device initialization
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (3 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 6/8] spapr: CPU hotplug support Bharata B Rao
` (4 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Initialize boot CPUs specified with -smp option as CPU core devices.
Create as many CPU package devices as necessary to fit in max_cpus and
populate (i,e., realize) only as many as required.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 31 +++++++++++++++++++++++--------
include/hw/ppc/spapr_cpu_package.h | 1 +
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3892a99..f3913b4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -64,6 +64,8 @@
#include "hw/compat.h"
#include "qemu-common.h"
+#include "hw/ppc/spapr_cpu_package.h"
+#include "monitor/monitor.h"
#include <libfdt.h>
@@ -1739,7 +1741,6 @@ static void ppc_spapr_init(MachineState *machine)
const char *kernel_filename = machine->kernel_filename;
const char *kernel_cmdline = machine->kernel_cmdline;
const char *initrd_filename = machine->initrd_filename;
- PowerPCCPU *cpu;
PCIHostState *phb;
int i;
MemoryRegion *sysmem = get_system_memory();
@@ -1753,6 +1754,8 @@ static void ppc_spapr_init(MachineState *machine)
long load_limit, fw_size;
bool kernel_le = false;
char *filename;
+ int spapr_packages = smp_cpus / smp_threads;
+ int spapr_max_packages = max_cpus / smp_threads;
msi_supported = true;
@@ -1825,13 +1828,18 @@ static void ppc_spapr_init(MachineState *machine)
}
machine->cpu_type = TYPE_POWERPC_CPU;
- for (i = 0; i < smp_cpus; i++) {
- cpu = cpu_ppc_init(machine->cpu_model);
- if (cpu == NULL) {
- error_report("Unable to find PowerPC CPU definition");
- exit(1);
+ /*
+ * Create enough CPU package devices for max_cpus and realize the
+ * required number of them.
+ */
+ for (i = 0; i < spapr_max_packages; i++) {
+ Object *spapr_cpu_package =
+ cpu_package_create_object(TYPE_SPAPR_CPU_PACKAGE, i, &error_fatal);
+
+ if (i < spapr_packages) {
+ object_property_set_bool(spapr_cpu_package, true, "realized",
+ &error_fatal);
}
- spapr_cpu_init(spapr, cpu, &error_fatal);
}
if (kvm_enabled()) {
@@ -2231,6 +2239,7 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
+ sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
int node;
@@ -2267,6 +2276,11 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
}
spapr_memory_plug(hotplug_dev, dev, node, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ CPUState *cs = CPU(dev);
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+ spapr_cpu_init(ms, cpu, errp);
}
}
@@ -2281,7 +2295,8 @@ static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine,
DeviceState *dev)
{
- if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+ object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
diff --git a/include/hw/ppc/spapr_cpu_package.h b/include/hw/ppc/spapr_cpu_package.h
index 547dbc1..a3810d8 100644
--- a/include/hw/ppc/spapr_cpu_package.h
+++ b/include/hw/ppc/spapr_cpu_package.h
@@ -10,6 +10,7 @@
#define HW_SPAPR_CPU_PACKAGE_H
#include "hw/qdev.h"
+#include "hw/cpu/package.h"
#define TYPE_SPAPR_CPU_PACKAGE "spapr-cpu-package"
#define SPAPR_CPU_PACKAGE(obj) \
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 6/8] spapr: CPU hotplug support
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (4 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 5/8] spapr: Convert boot CPUs into CPU core device initialization Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages Bharata B Rao
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Set up device tree entries for the hotplugged CPU core and use the
exising EPOW event infrastructure to send CPU hotplug notification to
the guest.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++-
hw/ppc/spapr_events.c | 3 ++
hw/ppc/spapr_rtas.c | 24 +++++++++
include/hw/ppc/spapr.h | 1 +
4 files changed, 163 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f3913b4..0bbbaf8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -604,6 +604,18 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
size_t page_sizes_prop_size;
uint32_t vcpus_per_socket = smp_threads * smp_cores;
uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
+ sPAPRDRConnector *drc;
+ sPAPRDRConnectorClass *drck;
+ int drc_index;
+
+ if (smc->dr_cpu_enabled) {
+ drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
+ g_assert(drc);
+ drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+ drc_index = drck->get_index(drc);
+ _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
+ }
/* Note: we keep CI large pages off for now because a 64K capable guest
* provisioned with large pages might otherwise try to map a qemu
@@ -988,6 +1000,16 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
_FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
}
+ if (smc->dr_cpu_enabled) {
+ int offset = fdt_path_offset(fdt, "/cpus");
+ ret = spapr_drc_populate_dt(fdt, offset, NULL,
+ SPAPR_DR_CONNECTOR_TYPE_CPU);
+ if (ret < 0) {
+ fprintf(stderr, "Couldn't set up CPU DR device tree properties\n");
+ exit(1);
+ }
+ }
+
_FDT((fdt_pack(fdt)));
if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
@@ -1756,6 +1778,7 @@ static void ppc_spapr_init(MachineState *machine)
char *filename;
int spapr_packages = smp_cpus / smp_threads;
int spapr_max_packages = max_cpus / smp_threads;
+ int smt = kvmppc_smt_threads();
msi_supported = true;
@@ -1822,6 +1845,15 @@ static void ppc_spapr_init(MachineState *machine)
spapr_validate_node_memory(machine, &error_fatal);
}
+ if (smc->dr_cpu_enabled) {
+ for (i = 0; i < spapr_max_packages; i++) {
+ sPAPRDRConnector *drc =
+ spapr_dr_connector_new(OBJECT(spapr),
+ SPAPR_DR_CONNECTOR_TYPE_CPU, i * smt);
+ qemu_register_reset(spapr_drc_reset, drc);
+ }
+ }
+
/* init CPUs */
if (machine->cpu_model == NULL) {
machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
@@ -2235,6 +2267,88 @@ out:
error_propagate(errp, local_err);
}
+static void *spapr_populate_hotplug_cpu_dt(DeviceState *dev, CPUState *cs,
+ int *fdt_offset,
+ sPAPRMachineState *spapr)
+{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ DeviceClass *dc = DEVICE_GET_CLASS(cs);
+ int id = ppc_get_vcpu_dt_id(cpu);
+ void *fdt;
+ int offset, fdt_size;
+ char *nodename;
+
+ fdt = create_device_tree(&fdt_size);
+ nodename = g_strdup_printf("%s@%x", dc->fw_name, id);
+ offset = fdt_add_subnode(fdt, 0, nodename);
+
+ spapr_populate_cpu_dt(cs, fdt, offset, spapr);
+ g_free(nodename);
+
+ *fdt_offset = offset;
+ return fdt;
+}
+
+static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
+ sPAPRMachineState *ms = SPAPR_MACHINE(qdev_get_machine());
+ sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(OBJECT(dev));
+ PowerPCCPU *cpu = package->thread0;
+ CPUState *cs = CPU(cpu);
+ int id = ppc_get_vcpu_dt_id(cpu);
+ sPAPRDRConnector *drc =
+ spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id);
+ sPAPRDRConnectorClass *drck;
+ Error *local_err = NULL;
+ void *fdt = NULL;
+ int fdt_offset = 0;
+
+ if (!smc->dr_cpu_enabled) {
+ /*
+ * This is a cold plugged CPU core but the machine doesn't support
+ * DR. So skip the hotplug path ensuring that the core is brought
+ * up online with out an associated DR connector.
+ */
+ return;
+ }
+
+ g_assert(drc);
+
+ /*
+ * Setup CPU DT entries only for hotplugged CPUs. For boot time or
+ * coldplugged CPUs DT entries are setup in spapr_finalize_fdt().
+ */
+ if (qdev_hotplug) {
+ fdt = spapr_populate_hotplug_cpu_dt(dev, cs, &fdt_offset, ms);
+ dev->hotplugged = true;
+ }
+
+ drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+ drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
+ if (local_err) {
+ g_free(fdt);
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ if (dev->hotplugged) {
+ /*
+ * Send hotplug notification interrupt to the guest only in case
+ * of hotplugged CPUs.
+ */
+ spapr_hotplug_req_add_by_index(drc);
+ } else {
+ /*
+ * Set the right DRC states for cold plugged CPU.
+ */
+ drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
+ drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
+ }
+ return;
+}
+
static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -2279,8 +2393,25 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
CPUState *cs = CPU(dev);
PowerPCCPU *cpu = POWERPC_CPU(cs);
+ int i;
+
+ if (!smc->dr_cpu_enabled && dev->hotplugged) {
+ error_setg(errp, "CPU hotplug not supported for this machine");
+ return;
+ }
+
+ /* Set NUMA node for the added CPUs */
+ for (i = 0; i < nb_numa_nodes; i++) {
+ if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) {
+ cs->numa_node = i;
+ break;
+ }
+ }
spapr_cpu_init(ms, cpu, errp);
+ spapr_cpu_reset(cpu);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_PACKAGE)) {
+ spapr_core_plug(hotplug_dev, dev, errp);
}
}
@@ -2296,7 +2427,8 @@ static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine,
DeviceState *dev)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
- object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
+ object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_PACKAGE)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
@@ -2340,6 +2472,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
smc->dr_lmb_enabled = true;
+ smc->dr_cpu_enabled = true;
fwc->get_dev_path = spapr_get_fw_dev_path;
nc->nmi_monitor_handler = spapr_nmi;
}
@@ -2419,6 +2552,7 @@ static void spapr_machine_2_5_class_options(MachineClass *mc)
spapr_machine_2_6_class_options(mc);
smc->use_ohci_by_default = true;
+ smc->dr_cpu_enabled = false;
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_5);
}
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index f5eac4b..e50bb16 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -437,6 +437,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
case SPAPR_DR_CONNECTOR_TYPE_LMB:
hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_MEMORY;
break;
+ case SPAPR_DR_CONNECTOR_TYPE_CPU:
+ hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_CPU;
+ break;
default:
/* we shouldn't be signaling hotplug events for resources
* that don't support them
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 07ad672..ed19466 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -34,6 +34,7 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
+#include "hw/ppc/ppc.h"
#include "qapi-event.h"
#include "hw/boards.h"
@@ -160,6 +161,27 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
}
+/*
+ * Set the timebase offset of the CPU to that of first CPU.
+ * This helps hotplugged CPU to have the correct timebase offset.
+ */
+static void spapr_cpu_update_tb_offset(PowerPCCPU *cpu)
+{
+ PowerPCCPU *fcpu = POWERPC_CPU(first_cpu);
+
+ cpu->env.tb_env->tb_offset = fcpu->env.tb_env->tb_offset;
+}
+
+static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
+{
+ PowerPCCPU *fcpu = POWERPC_CPU(first_cpu);
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(fcpu);
+
+ if (!pcc->interrupts_big_endian(fcpu)) {
+ cpu->env.spr[SPR_LPCR] |= LPCR_ILE;
+ }
+}
+
static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
uint32_t token, uint32_t nargs,
target_ulong args,
@@ -196,6 +218,8 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
env->nip = start;
env->gpr[3] = r3;
cs->halted = 0;
+ spapr_cpu_set_endianness(cpu);
+ spapr_cpu_update_tb_offset(cpu);
qemu_cpu_kick(cs);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1f9e722..a9d98e7 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -36,6 +36,7 @@ struct sPAPRMachineClass {
/*< public >*/
bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
+ bool dr_cpu_enabled; /* enable dynamic-reconfig/hotplug of CPUs */
bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
};
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (5 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 6/8] spapr: CPU hotplug support Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 16:49 ` Eric Blake
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 8/8] hmp: Implement 'info cpu-slots' Bharata B Rao
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/cpu/package.c | 19 +++++++++++++
hw/ppc/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/boards.h | 1 +
qapi-schema.json | 48 ++++++++++++++++++++++++++++++++
4 files changed, 147 insertions(+)
diff --git a/hw/cpu/package.c b/hw/cpu/package.c
index 259dbfa..4ff20fa 100644
--- a/hw/cpu/package.c
+++ b/hw/cpu/package.c
@@ -7,7 +7,26 @@
* See the COPYING file in the top-level directory.
*/
#include "hw/cpu/package.h"
+#include "hw/boards.h"
#include "qom/object_interfaces.h"
+#include "qmp-commands.h"
+#include "qapi/qmp/qerror.h"
+
+/*
+ * QMP: query cpu-pacakges
+ */
+CPUPackageInfoList *qmp_query_cpu_packages(Error **errp)
+{
+ MachineState *ms = MACHINE(qdev_get_machine());
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+ if (!mc->cpu_packages) {
+ error_setg(errp, QERR_UNSUPPORTED);
+ return NULL;
+ }
+
+ return mc->cpu_packages(ms);
+}
Object *cpu_package_create_object(const char *typename, uint32_t index,
Error **errp)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0bbbaf8..147b9d1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2441,6 +2441,84 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
return cpu_index / smp_threads / smp_cores;
}
+static int spapr_cpuinfo_list(Object *obj, void *opaque)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+ CPUInfoList ***prev = opaque;
+
+ if (object_dynamic_cast(obj, TYPE_CPU)) {
+ CPUInfoList *elem = g_new0(CPUInfoList, 1);
+ CPUInfo *s = g_new0(CPUInfo, 1);
+ CPUState *cpu = CPU(obj);
+ PowerPCCPU *pcpu = POWERPC_CPU(cpu);
+
+ s->arch_id = ppc_get_vcpu_dt_id(pcpu);
+ s->type = g_strdup(object_get_typename(obj));
+ s->thread = cpu->cpu_index;
+ s->has_thread = true;
+ s->core = cpu->cpu_index / smp_threads;
+ s->has_core = true;
+ if (mc->cpu_index_to_socket_id) {
+ s->socket = mc->cpu_index_to_socket_id(cpu->cpu_index);
+ } else {
+ s->socket = cpu->cpu_index / smp_threads / smp_cores;
+ }
+ s->has_socket = true;
+ s->node = cpu->numa_node;
+ s->has_node = true;
+ s->qom_path = object_get_canonical_path(obj);
+
+ elem->value = s;
+ elem->next = NULL;
+ **prev = elem;
+ *prev = &elem->next;
+ }
+ object_child_foreach(obj, spapr_cpuinfo_list, opaque);
+ return 0;
+}
+
+static int spapr_cpu_packageinfo_list(Object *obj, void *opaque)
+{
+ CPUPackageInfoList ***prev = opaque;
+
+ if (object_dynamic_cast(obj, TYPE_CPU_PACKAGE)) {
+ DeviceState *dev = DEVICE(obj);
+ CPUPackageInfoList *elem = g_new0(CPUPackageInfoList, 1);
+ CPUPackageInfo *s = g_new0(CPUPackageInfo, 1);
+ CPUInfoList *cpu_head = NULL;
+ CPUInfoList **cpu_prev = &cpu_head;
+
+ if (dev->id) {
+ s->has_id = true;
+ s->id = g_strdup(dev->id);
+ }
+ s->realized = object_property_get_bool(obj, "realized", NULL);
+ s->nr_cpus = smp_threads;
+ s->qom_path = object_get_canonical_path(obj);
+ s->type = g_strdup(TYPE_SPAPR_CPU_PACKAGE);
+ if (s->realized) {
+ spapr_cpuinfo_list(obj, &cpu_prev);
+ }
+ s->cpus = cpu_head;
+ elem->value = s;
+ elem->next = NULL;
+ **prev = elem;
+ *prev = &elem->next;
+ }
+
+ object_child_foreach(obj, spapr_cpu_packageinfo_list, opaque);
+ return 0;
+}
+
+static CPUPackageInfoList *spapr_cpu_packages(MachineState *machine)
+{
+ CPUPackageInfoList *head = NULL;
+ CPUPackageInfoList **prev = &head;
+
+ spapr_cpu_packageinfo_list(qdev_get_machine(), &prev);
+ return head;
+}
+
static void spapr_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -2467,6 +2545,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
mc->has_dynamic_sysbus = true;
mc->pci_allow_0_address = true;
mc->get_hotplug_handler = spapr_get_hotpug_handler;
+ mc->cpu_packages = spapr_cpu_packages;
hc->plug = spapr_machine_device_plug;
hc->unplug = spapr_machine_device_unplug;
mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index cf95d10..66d8780 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -99,6 +99,7 @@ struct MachineClass {
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+ CPUPackageInfoList *(*cpu_packages)(MachineState *machine);
};
/**
diff --git a/qapi-schema.json b/qapi-schema.json
index 8d04897..5a0dd80 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4083,3 +4083,51 @@
##
{ 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @CPUThreadInfo:
+#
+# Information about CPU Threads
+#
+# Since: 2.6
+##
+
+{ 'struct': 'CPUInfo',
+ 'data': { 'arch_id': 'int',
+ 'type': 'str',
+ '*thread': 'int',
+ '*core': 'int',
+ '*socket' : 'int',
+ '*node' : 'int',
+ '*qom_path': 'str'
+ }
+}
+
+##
+# @CPUPackageInfo:
+#
+# Information about CPU Packages
+#
+# Since: 2.6
+##
+
+{ 'struct': 'CPUPackageInfo',
+ 'data': { '*id': 'str',
+ 'type': 'str',
+ 'qom_path': 'str',
+ 'realized': 'bool',
+ 'nr_cpus': 'int',
+ 'cpus' : ['CPUInfo']
+ }
+}
+
+##
+# @query-cpu-packages:
+#
+# Returns information for all CPU packages
+#
+# Returns: a list of @CPUPackageInfo
+#
+# Since: 2.6
+##
+{ 'command': 'query-cpu-packages', 'returns': ['CPUPackageInfo'] }
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v0 8/8] hmp: Implement 'info cpu-slots'
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (6 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages Bharata B Rao
@ 2016-02-22 5:01 ` Bharata B Rao
2016-02-22 5:10 ` [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
2016-02-22 15:32 ` Andreas Färber
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:01 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, Bharata B Rao, agraf, armbru, pbonzini, imammedo,
afaerber, david
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hmp-commands-info.hx | 14 ++++++++++++++
hmp.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 1 +
3 files changed, 65 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 9b71351..6ca6da3 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -786,6 +786,20 @@ STEXI
Display the value of a storage key (s390 only)
ETEXI
+ {
+ .name = "cpu-packages",
+ .args_type = "",
+ .params = "",
+ .help = "show CPU packages",
+ .mhandler.cmd = hmp_info_cpu_packages,
+ },
+
+STEXI
+@item info cpu-packages
+@findex cpu-packages
+Show CPU packages
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index cb03a15..3450600 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2374,3 +2374,53 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
qapi_free_RockerOfDpaGroupList(list);
}
+
+void hmp_info_cpu_packages(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ CPUPackageInfoList *cpu_packageinfo_list = qmp_query_cpu_packages(&err);
+ CPUPackageInfoList *s = cpu_packageinfo_list;
+ CPUInfoList *cpu;
+ int i;
+
+ while (s) {
+ monitor_printf(mon, "CPU Package: \"%s\"\n",
+ s->value->has_id ? s->value->id : "");
+ monitor_printf(mon, " type: \"%s\"\n", s->value->type);
+ monitor_printf(mon, " qom_path: \"%s\"\n", s->value->qom_path);
+ monitor_printf(mon, " realized: %s\n",
+ s->value->realized ? "true" : "false");
+ monitor_printf(mon, " nr_cpus: %" PRId64 "\n", s->value->nr_cpus);
+ if (s->value->nr_cpus) {
+ for (i = 0, cpu = s->value->cpus; cpu; cpu = cpu->next, i++) {
+ monitor_printf(mon, " CPU: %" PRId32 "\n", i);
+ monitor_printf(mon, " Type: \"%s\"\n", cpu->value->type);
+ monitor_printf(mon, " Arch ID: %" PRId64 "\n",
+ cpu->value->arch_id);
+ if (cpu->value->has_thread) {
+ monitor_printf(mon, " Thread: %" PRId64 "\n",
+ cpu->value->thread);
+ }
+ if (cpu->value->has_core) {
+ monitor_printf(mon, " Core: %" PRId64 "\n",
+ cpu->value->core);
+ }
+ if (cpu->value->has_core) {
+ monitor_printf(mon, " Socket: %" PRId64 "\n",
+ cpu->value->socket);
+ }
+ if (cpu->value->has_core) {
+ monitor_printf(mon, " Node: %" PRId64 "\n",
+ cpu->value->node);
+ }
+ if (cpu->value->has_qom_path) {
+ monitor_printf(mon, " qom_path: \"%s\"\n",
+ cpu->value->qom_path);
+ }
+ }
+ }
+ s = s->next;
+ }
+
+ qapi_free_CPUPackageInfoList(cpu_packageinfo_list);
+}
diff --git a/hmp.h b/hmp.h
index a8c5b5a..f78f55f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -131,5 +131,6 @@ void hmp_rocker(Monitor *mon, const QDict *qdict);
void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
+void hmp_info_cpu_packages(Monitor *mon, const QDict *qdict);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (7 preceding siblings ...)
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 8/8] hmp: Implement 'info cpu-slots' Bharata B Rao
@ 2016-02-22 5:10 ` Bharata B Rao
2016-02-22 15:32 ` Andreas Färber
9 siblings, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 5:10 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, aik, agraf, armbru, pbonzini, imammedo, afaerber, david
On Mon, Feb 22, 2016 at 10:31:17AM +0530, Bharata B Rao wrote:
> Hi,
>
> This is an attempt to implement David Gibson's RFC that was posted at
> https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html
> I am not sure if I have followed all the aspects of the RFC fully, but we
> can make changes going forward.
>
> An example cpu-package implementation is done for sPAPR in this patchset.
> Hot removal is not yet done in this patchset.
>
> For the command line,
>
> -smp 8,sockets=1,cores=1,threads=8,maxcpus=16 -numa node,nodeid=0,cpus=0-7 -numa node,nodeid=1,cpus=8-15
>
> the HMP query looks like this:
>
> (qemu) info cpu-packages
> CPU Package: ""
> type: "spapr-cpu-package"
> qom_path: "/machine/cpu-package[0]"
> realized: true
> nr_cpus: 8
> CPU: 0
> Type: "host-powerpc64-cpu"
> Arch ID: 0
> Thread: 0
> Core: 0
> Socket: 0
> Node: 0
qom_path: "/machine/cpu-package[0]/thread[0]"
Missed qom_path.
Regards,
Bharata.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device Bharata B Rao
@ 2016-02-22 6:44 ` Andreas Färber
2016-02-22 7:47 ` David Gibson
2016-02-22 8:05 ` Bharata B Rao
0 siblings, 2 replies; 22+ messages in thread
From: Andreas Färber @ 2016-02-22 6:44 UTC (permalink / raw)
To: Bharata B Rao, qemu-devel
Cc: ehabkost, aik, agraf, armbru, pbonzini, imammedo, david
Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
> performed in the granularity of CPU core device by setting the "realized"
> property of this device to "true". When hotplugged, CPU core creates CPU
> thread devices.
>
> TODO: Right now allows for only homogeneous configurations as we depend
> on global smp_threads and machine->cpu_model.
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
> hw/ppc/Makefile.objs | 1 +
> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
> 3 files changed, 77 insertions(+)
> create mode 100644 hw/ppc/spapr_cpu_package.c
> create mode 100644 include/hw/ppc/spapr_cpu_package.h
>
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index c1ffc77..3000982 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> obj-y += spapr_pci_vfio.o
> endif
> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
> new file mode 100644
> index 0000000..3120a16
> --- /dev/null
> +++ b/hw/ppc/spapr_cpu_package.c
> @@ -0,0 +1,50 @@
> +/*
> + * sPAPR CPU package device, acts as container of CPU thread devices.
> + *
> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "hw/cpu/package.h"
> +#include "hw/ppc/spapr_cpu_package.h"
> +#include "hw/boards.h"
> +#include <sysemu/cpus.h>
> +#include "qemu/error-report.h"
> +
> +static void spapr_cpu_package_instance_init(Object *obj)
> +{
> + int i;
> + CPUState *cpu;
> + MachineState *machine = MACHINE(qdev_get_machine());
> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
> +
> + /* Create as many CPU threads as specified in the topology */
> + for (i = 0; i < smp_threads; i++) {
> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
No, no, no. This is horribly violating QOM design.
Please compare the x86 RFC.
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 6:44 ` Andreas Färber
@ 2016-02-22 7:47 ` David Gibson
2016-02-22 15:58 ` Andreas Färber
2016-02-22 8:05 ` Bharata B Rao
1 sibling, 1 reply; 22+ messages in thread
From: David Gibson @ 2016-02-22 7:47 UTC (permalink / raw)
To: Andreas Färber
Cc: ehabkost, aik, armbru, Bharata B Rao, agraf, qemu-devel,
pbonzini, imammedo
[-- Attachment #1: Type: text/plain, Size: 3557 bytes --]
On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote:
> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> > sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
> > performed in the granularity of CPU core device by setting the "realized"
> > property of this device to "true". When hotplugged, CPU core creates CPU
> > thread devices.
> >
> > TODO: Right now allows for only homogeneous configurations as we depend
> > on global smp_threads and machine->cpu_model.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> > hw/ppc/Makefile.objs | 1 +
> > hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
> > 3 files changed, 77 insertions(+)
> > create mode 100644 hw/ppc/spapr_cpu_package.c
> > create mode 100644 include/hw/ppc/spapr_cpu_package.h
> >
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index c1ffc77..3000982 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
> > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> > +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
> > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> > obj-y += spapr_pci_vfio.o
> > endif
> > diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
> > new file mode 100644
> > index 0000000..3120a16
> > --- /dev/null
> > +++ b/hw/ppc/spapr_cpu_package.c
> > @@ -0,0 +1,50 @@
> > +/*
> > + * sPAPR CPU package device, acts as container of CPU thread devices.
> > + *
> > + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "hw/cpu/package.h"
> > +#include "hw/ppc/spapr_cpu_package.h"
> > +#include "hw/boards.h"
> > +#include <sysemu/cpus.h>
> > +#include "qemu/error-report.h"
> > +
> > +static void spapr_cpu_package_instance_init(Object *obj)
> > +{
> > + int i;
> > + CPUState *cpu;
> > + MachineState *machine = MACHINE(qdev_get_machine());
> > + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
> > +
> > + /* Create as many CPU threads as specified in the topology */
> > + for (i = 0; i < smp_threads; i++) {
> > + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
>
> No, no, no. This is horribly violating QOM design.
Ok.. why? There does not, to me, seem to be any remotely easily
discoverable means of finding out what QOM design principles are.
It also would have been nice if you weighed in on my RFC this code is
based on.
> Please compare the x86 RFC.
Where do I find this?
From all I could tell there just seemed to be a lot of spinning of
wheels on the cpu hotplug stuff, which is why I made the cpu-packages
proposal to try and move things forward.
There's been a lot of "don't do it like that" but precious little
"here's how you _should_ do it" that actually addresses the needs of
the various platforms.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState Bharata B Rao
@ 2016-02-22 8:04 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2016-02-22 8:04 UTC (permalink / raw)
To: Bharata B Rao
Cc: ehabkost, aik, armbru, qemu-devel, agraf, pbonzini, imammedo, afaerber
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
On Mon, Feb 22, 2016 at 10:31:18AM +0530, Bharata B Rao wrote:
> Storing CPU typename in MachineState lets us to create CPU threads
> for all architectures in uniform manner from arch-neutral code.
>
> TODO: Touching only sPAPR target for now
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
As noted elsewhere, I hope we can do away with the need for this, but
I haven't yet figured out enough about the QOM model to see how.
> ---
> hw/ppc/spapr.c | 2 ++
> include/hw/boards.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5bd8fd3..3892a99 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1823,6 +1823,8 @@ static void ppc_spapr_init(MachineState *machine)
> if (machine->cpu_model == NULL) {
> machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
> }
> + machine->cpu_type = TYPE_POWERPC_CPU;
> +
> for (i = 0; i < smp_cpus; i++) {
> cpu = cpu_ppc_init(machine->cpu_model);
> if (cpu == NULL) {
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 0f30959..cf95d10 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -137,6 +137,7 @@ struct MachineState {
> char *kernel_cmdline;
> char *initrd_filename;
> const char *cpu_model;
> + const char *cpu_type;
> AccelState *accelerator;
> };
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 6:44 ` Andreas Färber
2016-02-22 7:47 ` David Gibson
@ 2016-02-22 8:05 ` Bharata B Rao
2016-02-22 15:48 ` Andreas Färber
1 sibling, 1 reply; 22+ messages in thread
From: Bharata B Rao @ 2016-02-22 8:05 UTC (permalink / raw)
To: Andreas Färber
Cc: ehabkost, aik, armbru, agraf, qemu-devel, pbonzini, imammedo, david
On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote:
> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> > sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
> > performed in the granularity of CPU core device by setting the "realized"
> > property of this device to "true". When hotplugged, CPU core creates CPU
> > thread devices.
> >
> > TODO: Right now allows for only homogeneous configurations as we depend
> > on global smp_threads and machine->cpu_model.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> > hw/ppc/Makefile.objs | 1 +
> > hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
> > 3 files changed, 77 insertions(+)
> > create mode 100644 hw/ppc/spapr_cpu_package.c
> > create mode 100644 include/hw/ppc/spapr_cpu_package.h
> >
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index c1ffc77..3000982 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
> > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> > +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
> > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> > obj-y += spapr_pci_vfio.o
> > endif
> > diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
> > new file mode 100644
> > index 0000000..3120a16
> > --- /dev/null
> > +++ b/hw/ppc/spapr_cpu_package.c
> > @@ -0,0 +1,50 @@
> > +/*
> > + * sPAPR CPU package device, acts as container of CPU thread devices.
> > + *
> > + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "hw/cpu/package.h"
> > +#include "hw/ppc/spapr_cpu_package.h"
> > +#include "hw/boards.h"
> > +#include <sysemu/cpus.h>
> > +#include "qemu/error-report.h"
> > +
> > +static void spapr_cpu_package_instance_init(Object *obj)
> > +{
> > + int i;
> > + CPUState *cpu;
> > + MachineState *machine = MACHINE(qdev_get_machine());
> > + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
> > +
> > + /* Create as many CPU threads as specified in the topology */
> > + for (i = 0; i < smp_threads; i++) {
> > + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
>
> No, no, no. This is horribly violating QOM design.
>
> Please compare the x86 RFC.
Are you referring to the in-place initialization of child objects like
that you did in socket based x86 hotplug RFC last year ?
Regards,
Bharata.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
` (8 preceding siblings ...)
2016-02-22 5:10 ` [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
@ 2016-02-22 15:32 ` Andreas Färber
2016-02-22 23:28 ` David Gibson
2016-02-23 6:11 ` Bharata B Rao
9 siblings, 2 replies; 22+ messages in thread
From: Andreas Färber @ 2016-02-22 15:32 UTC (permalink / raw)
To: Bharata B Rao, qemu-devel
Cc: ehabkost, aik, agraf, armbru, pbonzini, imammedo, david
Hi Bharata,
Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> This is an attempt to implement David Gibson's RFC that was posted at
> https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html
> I am not sure if I have followed all the aspects of the RFC fully, but we
> can make changes going forward.
I am not familiar with David's RFC beyond what was portrayed on the KVM
call - this is not what we discussed on the call and I don't like it.
Further, your commits are pretty cryptic to me. Please improve your
commit messages.
For example, you add a cpu_type field and you assign it the value
TYPE_POWERPC_CPU. That's not the user-chosen CPU type then, it's a base
CPU type that cannot be instantiated. Either name it cpu_base_type or
fill it in with proper values in one patch - that patch on its own does
not create value and does not explain your claim:
"Storing CPU typename in MachineState lets us to create CPU threads
for all architectures in uniform manner from arch-neutral code."
I'm pretty sure that CPU threads cannot be created from that type, as it
would run into an assertion.
Next, you make a functionally correct refactoring of cpu_generic_init(),
but I don't understand why you duplicate that code. cpu_foo_init() still
expects things to be realized, so instead of realizing once in a central
place you do it in nine different places. Had you touched all helper
functions we might be able to move that to three places, once for
softmmu, once or twice for linux-user and once for bsd-user. But I
rather get the feeling that you misunderstand those legacy helper
functions, they're for -cpu handling and not to my knowledge used for
cpu-add at all. You should not be using them and then won't need to
touch them in this way. By using them in your supposedly QOM code you
are hiding an object_new() call inside deep layers of helper functions
instead of using QOM native functions such as object_initialize(),
object_new() and object_property_set*().
Is "CPU package" some IBM sPAPR term? It is new to me and does not match
-smp precedence, so I really don't think we should be forcing that term
on all architectures for no good reason.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 8:05 ` Bharata B Rao
@ 2016-02-22 15:48 ` Andreas Färber
0 siblings, 0 replies; 22+ messages in thread
From: Andreas Färber @ 2016-02-22 15:48 UTC (permalink / raw)
To: bharata
Cc: ehabkost, aik, armbru, agraf, qemu-devel, pbonzini, imammedo, david
Am 22.02.2016 um 09:05 schrieb Bharata B Rao:
> On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote:
>> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
>>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
>>> performed in the granularity of CPU core device by setting the "realized"
>>> property of this device to "true". When hotplugged, CPU core creates CPU
>>> thread devices.
>>>
>>> TODO: Right now allows for only homogeneous configurations as we depend
>>> on global smp_threads and machine->cpu_model.
>>>
>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> ---
>>> hw/ppc/Makefile.objs | 1 +
>>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
>>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
>>> 3 files changed, 77 insertions(+)
>>> create mode 100644 hw/ppc/spapr_cpu_package.c
>>> create mode 100644 include/hw/ppc/spapr_cpu_package.h
>>>
>>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>>> index c1ffc77..3000982 100644
>>> --- a/hw/ppc/Makefile.objs
>>> +++ b/hw/ppc/Makefile.objs
>>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
>>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
>>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>>> obj-y += spapr_pci_vfio.o
>>> endif
>>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
>>> new file mode 100644
>>> index 0000000..3120a16
>>> --- /dev/null
>>> +++ b/hw/ppc/spapr_cpu_package.c
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + * sPAPR CPU package device, acts as container of CPU thread devices.
>>> + *
>>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + */
>>> +#include "hw/cpu/package.h"
>>> +#include "hw/ppc/spapr_cpu_package.h"
>>> +#include "hw/boards.h"
>>> +#include <sysemu/cpus.h>
>>> +#include "qemu/error-report.h"
>>> +
>>> +static void spapr_cpu_package_instance_init(Object *obj)
>>> +{
>>> + int i;
>>> + CPUState *cpu;
>>> + MachineState *machine = MACHINE(qdev_get_machine());
>>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
>>> +
>>> + /* Create as many CPU threads as specified in the topology */
>>> + for (i = 0; i < smp_threads; i++) {
>>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
>>
>> No, no, no. This is horribly violating QOM design.
>>
>> Please compare the x86 RFC.
>
> Are you referring to the in-place initialization of child objects like
> that you did in socket based x86 hotplug RFC last year ?
Yes, in-place object_initialize() is one of two options. The other is
using object_initialize() with g_try_malloc0() from property setters,
like Alexey did for xics I think.
But mainly the idea is to not bend your QOM types to reuse the legacy
glue but to use true QOM concepts in the design of those new objects.
If there is some big hurdle I'm missing, please say so openly.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 7:47 ` David Gibson
@ 2016-02-22 15:58 ` Andreas Färber
2016-02-22 23:24 ` David Gibson
0 siblings, 1 reply; 22+ messages in thread
From: Andreas Färber @ 2016-02-22 15:58 UTC (permalink / raw)
To: David Gibson
Cc: ehabkost, aik, armbru, Bharata B Rao, agraf, qemu-devel,
pbonzini, imammedo
Am 22.02.2016 um 08:47 schrieb David Gibson:
> On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote:
>> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
>>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
>>> performed in the granularity of CPU core device by setting the "realized"
>>> property of this device to "true". When hotplugged, CPU core creates CPU
>>> thread devices.
>>>
>>> TODO: Right now allows for only homogeneous configurations as we depend
>>> on global smp_threads and machine->cpu_model.
>>>
>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> ---
>>> hw/ppc/Makefile.objs | 1 +
>>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
>>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
>>> 3 files changed, 77 insertions(+)
>>> create mode 100644 hw/ppc/spapr_cpu_package.c
>>> create mode 100644 include/hw/ppc/spapr_cpu_package.h
>>>
>>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>>> index c1ffc77..3000982 100644
>>> --- a/hw/ppc/Makefile.objs
>>> +++ b/hw/ppc/Makefile.objs
>>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
>>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
>>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>>> obj-y += spapr_pci_vfio.o
>>> endif
>>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
>>> new file mode 100644
>>> index 0000000..3120a16
>>> --- /dev/null
>>> +++ b/hw/ppc/spapr_cpu_package.c
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + * sPAPR CPU package device, acts as container of CPU thread devices.
>>> + *
>>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + */
>>> +#include "hw/cpu/package.h"
>>> +#include "hw/ppc/spapr_cpu_package.h"
>>> +#include "hw/boards.h"
>>> +#include <sysemu/cpus.h>
>>> +#include "qemu/error-report.h"
>>> +
>>> +static void spapr_cpu_package_instance_init(Object *obj)
>>> +{
>>> + int i;
>>> + CPUState *cpu;
>>> + MachineState *machine = MACHINE(qdev_get_machine());
>>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
>>> +
>>> + /* Create as many CPU threads as specified in the topology */
>>> + for (i = 0; i < smp_threads; i++) {
>>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
>>
>> No, no, no. This is horribly violating QOM design.
>
> Ok.. why? There does not, to me, seem to be any remotely easily
> discoverable means of finding out what QOM design principles are.
>
> It also would have been nice if you weighed in on my RFC this code is
> based on.
It would've been nice had you joined our KVM call prompted by _your_
suggestions (which again you could've discussed at KVM Forum where I had
a session specifically for discussing this topic), but you didn't.
I did discuss several aspects on the call and in the recorded session
and will not write it by email again. Feel free to put it on the call
agenda again.
I told Bharata that you can't have a base type that suddenly mutates its
topology, therefore we discussed the possibility of having a QOM
_interface_, not a base type as apparently done here. We deliberately do
not have multi-inheritence in QOM; there's not just ppc in the world.
My time for reading list mails is limited. Make it easy for me to
understand what the difference is between your package and my core and
why instead of reusing my posted cpu-core type you need to propose your
own cpu-package type. In one point you are right, we keep going in
circles and sometimes backwards rather than forward here.
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages Bharata B Rao
@ 2016-02-22 16:49 ` Eric Blake
2016-02-23 8:37 ` Igor Mammedov
0 siblings, 1 reply; 22+ messages in thread
From: Eric Blake @ 2016-02-22 16:49 UTC (permalink / raw)
To: Bharata B Rao, qemu-devel
Cc: ehabkost, aik, armbru, agraf, pbonzini, imammedo, afaerber, david
[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]
On 02/21/2016 10:01 PM, Bharata B Rao wrote:
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
> hw/cpu/package.c | 19 +++++++++++++
> hw/ppc/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/boards.h | 1 +
> qapi-schema.json | 48 ++++++++++++++++++++++++++++++++
> 4 files changed, 147 insertions(+)
>
> +++ b/qapi-schema.json
> @@ -4083,3 +4083,51 @@
> ##
> { 'enum': 'ReplayMode',
> 'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @CPUThreadInfo:
> +#
> +# Information about CPU Threads
> +#
Missing documentation for the fields.
> +# Since: 2.6
> +##
> +
> +{ 'struct': 'CPUInfo',
> + 'data': { 'arch_id': 'int',
New QMP code should favor '-' over '_'; this should be 'arch-id'.
> + 'type': 'str',
Is this string free-form, or is it a finite set of values? If the
latter, then it should be an enum type.
> + '*thread': 'int',
> + '*core': 'int',
> + '*socket' : 'int',
> + '*node' : 'int',
> + '*qom_path': 'str'
'qom-path'. But this one is definitely free-form, so 'str' is right.
> + }
> +}
> +
> +##
> +# @CPUPackageInfo:
> +#
> +# Information about CPU Packages
> +#
Missing field documentation.
> +# Since: 2.6
> +##
> +
> +{ 'struct': 'CPUPackageInfo',
> + 'data': { '*id': 'str',
> + 'type': 'str',
> + 'qom_path': 'str',
> + 'realized': 'bool',
> + 'nr_cpus': 'int',
'nr-cpus'. Is this field redundant, given that the caller can just count
the length of the 'cpus' array?
> + 'cpus' : ['CPUInfo']
> + }
> +}
> +
> +##
> +# @query-cpu-packages:
> +#
> +# Returns information for all CPU packages
> +#
> +# Returns: a list of @CPUPackageInfo
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'query-cpu-packages', 'returns': ['CPUPackageInfo'] }
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device
2016-02-22 15:58 ` Andreas Färber
@ 2016-02-22 23:24 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2016-02-22 23:24 UTC (permalink / raw)
To: Andreas Färber
Cc: ehabkost, aik, armbru, Bharata B Rao, agraf, qemu-devel,
pbonzini, imammedo
[-- Attachment #1: Type: text/plain, Size: 6396 bytes --]
On Mon, Feb 22, 2016 at 04:58:46PM +0100, Andreas Färber wrote:
> Am 22.02.2016 um 08:47 schrieb David Gibson:
> > On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote:
> >> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> >>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is
> >>> performed in the granularity of CPU core device by setting the "realized"
> >>> property of this device to "true". When hotplugged, CPU core creates CPU
> >>> thread devices.
> >>>
> >>> TODO: Right now allows for only homogeneous configurations as we depend
> >>> on global smp_threads and machine->cpu_model.
> >>>
> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> >>> ---
> >>> hw/ppc/Makefile.objs | 1 +
> >>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++
> >>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++
> >>> 3 files changed, 77 insertions(+)
> >>> create mode 100644 hw/ppc/spapr_cpu_package.c
> >>> create mode 100644 include/hw/ppc/spapr_cpu_package.h
> >>>
> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> >>> index c1ffc77..3000982 100644
> >>> --- a/hw/ppc/Makefile.objs
> >>> +++ b/hw/ppc/Makefile.objs
> >>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
> >>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> >>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> >>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> >>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o
> >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >>> obj-y += spapr_pci_vfio.o
> >>> endif
> >>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c
> >>> new file mode 100644
> >>> index 0000000..3120a16
> >>> --- /dev/null
> >>> +++ b/hw/ppc/spapr_cpu_package.c
> >>> @@ -0,0 +1,50 @@
> >>> +/*
> >>> + * sPAPR CPU package device, acts as container of CPU thread devices.
> >>> + *
> >>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com>
> >>> + *
> >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >>> + * See the COPYING file in the top-level directory.
> >>> + */
> >>> +#include "hw/cpu/package.h"
> >>> +#include "hw/ppc/spapr_cpu_package.h"
> >>> +#include "hw/boards.h"
> >>> +#include <sysemu/cpus.h>
> >>> +#include "qemu/error-report.h"
> >>> +
> >>> +static void spapr_cpu_package_instance_init(Object *obj)
> >>> +{
> >>> + int i;
> >>> + CPUState *cpu;
> >>> + MachineState *machine = MACHINE(qdev_get_machine());
> >>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj);
> >>> +
> >>> + /* Create as many CPU threads as specified in the topology */
> >>> + for (i = 0; i < smp_threads; i++) {
> >>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model);
> >>
> >> No, no, no. This is horribly violating QOM design.
> >
> > Ok.. why? There does not, to me, seem to be any remotely easily
> > discoverable means of finding out what QOM design principles are.
> >
> > It also would have been nice if you weighed in on my RFC this code is
> > based on.
So, first, apologies for my grumpy tone. This is just one of a number
of frustrations I've had in the last few days, putting me in an
uncharitable frame of mind.
> It would've been nice had you joined our KVM call prompted by _your_
> suggestions
Ok, a) I was not aware that this call was prompted by my suggestions.
I got some vague second hand notices about it; as I do about KVM calls
from time to time, which I ignored, as usual, because b) the KVM call
is at stupid o'clock for me.
> (which again you could've discussed at KVM Forum where I had
> a session specifically for discussing this topic), but you didn't.
At KVM Forum, cpu hotplug wasn't yet on my radar.
> I did discuss several aspects on the call and in the recorded session
> and will not write it by email again. Feel free to put it on the call
> agenda again.
>
> I told Bharata that you can't have a base type that suddenly mutates its
> topology,
Ok, I'm not entirely sure what you mean by that. Do you mean the fact
that the core object constructs its own sub-objects?
> therefore we discussed the possibility of having a QOM
> _interface_, not a base type as apparently done here. We deliberately do
> not have multi-inheritence in QOM; there's not just ppc in the world.
So, in other discussions I've realised the package needs to be an
interface, rather than a type. But the patch your objecting to here
implements the spapr-core subtype. If that were implementing rather
than inheriting "cpu-package" I don't see that it would really change
the logic here.
Like you (I think) I do dislike the use of machine->cpu_type and
machine->cpu_model. I just haven't figured out enough QOM to know how
to avoid them. What is the QOM equivalent of, essentially, arguments
to a constructor?
> My time for reading list mails is limited. Make it easy for me to
> understand what the difference is between your package and my core and
> why instead of reusing my posted cpu-core type you need to propose your
Because my time for reading the list is also limited, so I haven't
seen your cpu-core posting. I did see Bharata's earlier core based
stuff which didn't seem to go over well either.
> own cpu-package type. In one point you are right, we keep going in
> circles and sometimes backwards rather than forward here.
But, also, from what I can tell of those parts of the discussion I
have seen, locking the hotplug at core level doesn't seem to work. It
seems to create an awful backwards compatibility tangle for x86 which
has existing thread-level hotplug, and I expect it would cause
problems when we encounter a platform with socket level hotplug only
(this seems very plausible for something modelling physical hotplug).
The idea of cpu-package was to abstract away the granularity of cpu
hotplug from a fixed level of the socket/core/thread heirarchy, while
still making that granularity easy to discover for management.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug
2016-02-22 15:32 ` Andreas Färber
@ 2016-02-22 23:28 ` David Gibson
2016-02-23 6:11 ` Bharata B Rao
1 sibling, 0 replies; 22+ messages in thread
From: David Gibson @ 2016-02-22 23:28 UTC (permalink / raw)
To: Andreas Färber
Cc: ehabkost, aik, armbru, Bharata B Rao, agraf, qemu-devel,
pbonzini, imammedo
[-- Attachment #1: Type: text/plain, Size: 2838 bytes --]
On Mon, Feb 22, 2016 at 04:32:25PM +0100, Andreas Färber wrote:
> Hi Bharata,
>
> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> > This is an attempt to implement David Gibson's RFC that was posted at
> > https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html
> > I am not sure if I have followed all the aspects of the RFC fully, but we
> > can make changes going forward.
>
> I am not familiar with David's RFC beyond what was portrayed on the KVM
> call - this is not what we discussed on the call and I don't like it.
>
> Further, your commits are pretty cryptic to me. Please improve your
> commit messages.
>
> For example, you add a cpu_type field and you assign it the value
> TYPE_POWERPC_CPU. That's not the user-chosen CPU type then, it's a base
> CPU type that cannot be instantiated. Either name it cpu_base_type or
> fill it in with proper values in one patch - that patch on its own does
> not create value and does not explain your claim:
> "Storing CPU typename in MachineState lets us to create CPU threads
> for all architectures in uniform manner from arch-neutral code."
> I'm pretty sure that CPU threads cannot be created from that type, as it
> would run into an assertion.
>
> Next, you make a functionally correct refactoring of cpu_generic_init(),
> but I don't understand why you duplicate that code. cpu_foo_init() still
> expects things to be realized, so instead of realizing once in a central
> place you do it in nine different places. Had you touched all helper
> functions we might be able to move that to three places, once for
> softmmu, once or twice for linux-user and once for bsd-user. But I
> rather get the feeling that you misunderstand those legacy helper
> functions, they're for -cpu handling and not to my knowledge used for
> cpu-add at all. You should not be using them and then won't need to
> touch them in this way. By using them in your supposedly QOM code you
> are hiding an object_new() call inside deep layers of helper functions
> instead of using QOM native functions such as object_initialize(),
> object_new() and object_property_set*().
>
> Is "CPU package" some IBM sPAPR term? It is new to me and does not match
> -smp precedence, so I really don't think we should be forcing that term
> on all architectures for no good reason.
No, it's not an spapr term. *By design* it doesn't match -smp
precedence, as noted elsewhere the point is to not lock the unit of
hotplug granularity to a fixed level of the -smp heirarchy, because
there doesn't seem to be a level there we can pick which works for all
platforms.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug
2016-02-22 15:32 ` Andreas Färber
2016-02-22 23:28 ` David Gibson
@ 2016-02-23 6:11 ` Bharata B Rao
1 sibling, 0 replies; 22+ messages in thread
From: Bharata B Rao @ 2016-02-23 6:11 UTC (permalink / raw)
To: Andreas Färber
Cc: ehabkost, aik, armbru, agraf, qemu-devel, pbonzini, imammedo, david
On Mon, Feb 22, 2016 at 04:32:25PM +0100, Andreas Färber wrote:
> Hi Bharata,
>
> Am 22.02.2016 um 06:01 schrieb Bharata B Rao:
> > This is an attempt to implement David Gibson's RFC that was posted at
> > https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html
> > I am not sure if I have followed all the aspects of the RFC fully, but we
> > can make changes going forward.
>
> I am not familiar with David's RFC beyond what was portrayed on the KVM
> call - this is not what we discussed on the call and I don't like it.
>
> Further, your commits are pretty cryptic to me. Please improve your
> commit messages.
>
> For example, you add a cpu_type field and you assign it the value
> TYPE_POWERPC_CPU. That's not the user-chosen CPU type then, it's a base
> CPU type that cannot be instantiated. Either name it cpu_base_type or
> fill it in with proper values in one patch - that patch on its own does
> not create value and does not explain your claim:
> "Storing CPU typename in MachineState lets us to create CPU threads
> for all architectures in uniform manner from arch-neutral code."
> I'm pretty sure that CPU threads cannot be created from that type, as it
> would run into an assertion.
Yes, that should have been called cpu_base_type.
>
> Next, you make a functionally correct refactoring of cpu_generic_init(),
> but I don't understand why you duplicate that code. cpu_foo_init() still
> expects things to be realized, so instead of realizing once in a central
> place you do it in nine different places. Had you touched all helper
> functions we might be able to move that to three places, once for
> softmmu, once or twice for linux-user and once for bsd-user. But I
> rather get the feeling that you misunderstand those legacy helper
> functions, they're for -cpu handling and not to my knowledge used for
> cpu-add at all. You should not be using them and then won't need to
> touch them in this way. By using them in your supposedly QOM code you
> are hiding an object_new() call inside deep layers of helper functions
> instead of using QOM native functions such as object_initialize(),
> object_new() and object_property_set*().
The intention was to re-use this generic sounding (cpu_generic_init)
routine with a bit of modification. But since it is a legacy helper
as you mention, we should be fine with using object_new() and other
QOM native functions directly.
Regards,
Bharata.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages
2016-02-22 16:49 ` Eric Blake
@ 2016-02-23 8:37 ` Igor Mammedov
0 siblings, 0 replies; 22+ messages in thread
From: Igor Mammedov @ 2016-02-23 8:37 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, ehabkost, aik, agraf, armbru, Bharata B Rao,
pbonzini, afaerber, david
On Mon, 22 Feb 2016 09:49:40 -0700
Eric Blake <eblake@redhat.com> wrote:
> On 02/21/2016 10:01 PM, Bharata B Rao wrote:
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> > hw/cpu/package.c | 19 +++++++++++++
> > hw/ppc/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/boards.h | 1 +
> > qapi-schema.json | 48 ++++++++++++++++++++++++++++++++
> > 4 files changed, 147 insertions(+)
> >
>
> > +++ b/qapi-schema.json
> > @@ -4083,3 +4083,51 @@
> > ##
> > { 'enum': 'ReplayMode',
> > 'data': [ 'none', 'record', 'play' ] }
> > +
> > +##
> > +# @CPUThreadInfo:
> > +#
> > +# Information about CPU Threads
> > +#
>
> Missing documentation for the fields.
>
> > +# Since: 2.6
> > +##
> > +
> > +{ 'struct': 'CPUInfo',
> > + 'data': { 'arch_id': 'int',
>
> New QMP code should favor '-' over '_'; this should be 'arch-id'.
>
> > + 'type': 'str',
>
> Is this string free-form, or is it a finite set of values? If the
> latter, then it should be an enum type.
I'd say it's a limited set of values. The same applies to other
targets. It's basically cpu_model translated into corresponding
QOM type name.
Is there a way to auto-generate this QAPI enum dynamically?
>
> > + '*thread': 'int',
> > + '*core': 'int',
> > + '*socket' : 'int',
> > + '*node' : 'int',
> > + '*qom_path': 'str'
>
> 'qom-path'. But this one is definitely free-form, so 'str' is right.
>
> > + }
> > +}
> > +
> > +##
> > +# @CPUPackageInfo:
> > +#
> > +# Information about CPU Packages
> > +#
>
> Missing field documentation.
>
> > +# Since: 2.6
> > +##
> > +
> > +{ 'struct': 'CPUPackageInfo',
> > + 'data': { '*id': 'str',
> > + 'type': 'str',
> > + 'qom_path': 'str',
> > + 'realized': 'bool',
> > + 'nr_cpus': 'int',
>
> 'nr-cpus'. Is this field redundant, given that the caller can just count
> the length of the 'cpus' array?
>
> > + 'cpus' : ['CPUInfo']
> > + }
> > +}
> > +
> > +##
> > +# @query-cpu-packages:
> > +#
> > +# Returns information for all CPU packages
> > +#
> > +# Returns: a list of @CPUPackageInfo
> > +#
> > +# Since: 2.6
> > +##
> > +{ 'command': 'query-cpu-packages', 'returns': ['CPUPackageInfo'] }
> >
>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-02-23 8:37 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 5:01 [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 1/8] cpu: Store CPU typename in MachineState Bharata B Rao
2016-02-22 8:04 ` David Gibson
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 2/8] cpu: Don't realize CPU from cpu_generic_init() Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 3/8] cpu: CPU package abstract device Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 4/8] spapr: Introduce CPU core device Bharata B Rao
2016-02-22 6:44 ` Andreas Färber
2016-02-22 7:47 ` David Gibson
2016-02-22 15:58 ` Andreas Färber
2016-02-22 23:24 ` David Gibson
2016-02-22 8:05 ` Bharata B Rao
2016-02-22 15:48 ` Andreas Färber
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 5/8] spapr: Convert boot CPUs into CPU core device initialization Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 6/8] spapr: CPU hotplug support Bharata B Rao
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages Bharata B Rao
2016-02-22 16:49 ` Eric Blake
2016-02-23 8:37 ` Igor Mammedov
2016-02-22 5:01 ` [Qemu-devel] [RFC PATCH v0 8/8] hmp: Implement 'info cpu-slots' Bharata B Rao
2016-02-22 5:10 ` [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Bharata B Rao
2016-02-22 15:32 ` Andreas Färber
2016-02-22 23:28 ` David Gibson
2016-02-23 6:11 ` Bharata B Rao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).