On Fri, 11 Mar 2016 10:24:35 +0530
Bharata B Rao <bhar...@linux.vnet.ibm.com> wrote:
> Add sPAPR specific CPU core device that is based on generic CPU core device.
> Creating this core device will result in creation of all the CPU thread
> devices that are part of this core.
>
> Introduce sPAPRMachineClass.dr_cpu_enabled to indicate support for
> CPU core hotplug. Initialize boot time CPUs as core deivces and prevent
> topologies that result in partially filled cores. Both of these are done
> only if CPU core hotplug is supported.
>
> Note: An unrelated change in the call to xics_system_init() is done
> in this patch as it makes sense to use the local variable smt introduced
> in this patch instead of kvmppc_smt_threads() call here.
>
> Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> ---
> hw/ppc/Makefile.objs | 1 +
> hw/ppc/spapr.c | 68 +++++++++++---
> hw/ppc/spapr_cpu_core.c | 199
> ++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 4 +
> include/hw/ppc/spapr_cpu_core.h | 28 ++++++
> 5 files changed, 287 insertions(+), 13 deletions(-)
> create mode 100644 hw/ppc/spapr_cpu_core.c
> create mode 100644 include/hw/ppc/spapr_cpu_core.h
>
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index c1ffc77..5cc6608 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_core.o
> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> obj-y += spapr_pci_vfio.o
> endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 64c4acc..cffe8c8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -64,6 +64,7 @@
>
> #include "hw/compat.h"
> #include "qemu-common.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>
> #include <libfdt.h>
>
> @@ -1180,7 +1181,7 @@ static void ppc_spapr_reset(void)
>
> }
>
> -static void spapr_cpu_reset(void *opaque)
> +void spapr_cpu_reset(void *opaque)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> PowerPCCPU *cpu = opaque;
> @@ -1614,8 +1615,11 @@ static void spapr_boot_set(void *opaque, const char
> *boot_device,
> machine->boot_order = g_strdup(boot_device);
> }
>
> -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> - Error **errp)
> +/*
> + * TODO: Check if some of these can be moved to rtas_start_cpu() where
> + * a few other things required for hotplugged CPUs are being done.
> + */
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> {
> CPUPPCState *env = &cpu->env;
>
> @@ -1728,7 +1732,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();
> @@ -1742,6 +1745,22 @@ static void ppc_spapr_init(MachineState *machine)
> long load_limit, fw_size;
> bool kernel_le = false;
> char *filename;
> + int smt = kvmppc_smt_threads();
> + int spapr_cores = smp_cpus / smp_threads;
> + int spapr_max_cores = max_cpus / smp_threads;
> +
> + if (smc->dr_cpu_enabled) {
> + if (smp_cpus % smp_threads) {
> + error_report("smp_cpus (%u) must be multiple of threads (%u)",
> + smp_cpus, smp_threads);
> + exit(1);
> + }
> + if (max_cpus % smp_threads) {
> + error_report("max_cpus (%u) must be multiple of threads (%u)",
> + max_cpus, smp_threads);
> + exit(1);
> + }
> + }
>
> msi_supported = true;
>
> @@ -1788,8 +1807,7 @@ static void ppc_spapr_init(MachineState *machine)
>
> /* Set up Interrupt Controller before we create the VCPUs */
> spapr->icp = xics_system_init(machine,
> - DIV_ROUND_UP(max_cpus *
> kvmppc_smt_threads(),
> - smp_threads),
> + DIV_ROUND_UP(max_cpus * smt, smp_threads),
is smt == smp_threads, if not then what's a difference?
> XICS_IRQS, &error_fatal);
>
> if (smc->dr_lmb_enabled) {
> @@ -1800,13 +1818,34 @@ static void ppc_spapr_init(MachineState *machine)
> if (machine->cpu_model == NULL) {
> machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
> }
> - 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);
> +
> + if (smc->dr_cpu_enabled) {
> + spapr->cores = g_new0(Object *, spapr_max_cores);
> +
> + for (i = 0; i < spapr_max_cores; i++) {
> + int core_dt_id = i * smt;
> +
> + if (i < spapr_cores) {
> + Object *core = object_new(TYPE_SPAPR_CPU_CORE);
> +
> + object_property_set_str(core, machine->cpu_model,
> "cpu_model",
> + &error_fatal);
> + object_property_set_int(core, smp_threads, "threads",
> + &error_fatal);
> + object_property_set_int(core, core_dt_id, CPU_CORE_PROP_CORE,
> + &error_fatal);
> + object_property_set_bool(core, true, "realized",
> &error_fatal);
> + }
> }
> - spapr_cpu_init(spapr, cpu, &error_fatal);
> + } else {
> + for (i = 0; i < smp_cpus; i++) {
> + PowerPCCPU *cpu = cpu_ppc_init(machine->cpu_model);
> + if (cpu == NULL) {
> + error_report("Unable to find PowerPC CPU definition");
> + exit(1);
> + }
> + spapr_cpu_init(spapr, cpu, &error_fatal);
> + }
> }
>
> if (kvm_enabled()) {
> @@ -2261,7 +2300,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_SPAPR_CPU_CORE)) {
> return HOTPLUG_HANDLER(machine);
> }
> return NULL;
> @@ -2305,6 +2345,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;
> }
> @@ -2384,6 +2425,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_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> new file mode 100644
> index 0000000..8c6d71d
> --- /dev/null
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -0,0 +1,199 @@
> +/*
> + * sPAPR CPU core device, acts as container of CPU thread devices.
> + *
> + * Copyright (C) 2016 Bharata B Rao <bhar...@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/core.h"
> +#include "hw/ppc/spapr_cpu_core.h"
> +#include "hw/ppc/spapr.h"
> +#include "hw/boards.h"
> +#include "qemu/error-report.h"
> +#include "qapi/visitor.h"
> +#include <sysemu/cpus.h>
> +#include "target-ppc/kvm_ppc.h"
> +
> +static void spapr_cpu_core_create_threads(sPAPRCPUCore *core, int threads,
> + Error **errp)
> +{
> + int i;
> + Error *local_err = NULL;
> +
> + for (i = 0; i < threads; i++) {
> + char id[32];
> +
> + object_initialize(&core->threads[i], sizeof(core->threads[i]),
> + object_class_get_name(core->oc));
> + snprintf(id, sizeof(id), "thread[%d]", i);
> + object_property_add_child(OBJECT(core), id,
> OBJECT(&core->threads[i]),
> + &local_err);
> + if (local_err) {
> + goto err;
> + }
> + }
> + return;
> +
> +err:
> + while (--i) {
> + object_unparent(OBJECT(&core->threads[i]));
> + }
> + error_propagate(errp, local_err);
> +}
> +
> +static int spapr_cpu_core_realize_child(Object *child, void *opaque)
> +{
> + Error **errp = opaque;
> + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + CPUState *cs = CPU(child);
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> + object_property_set_bool(child, true, "realized", errp);
> + if (*errp) {
> + return 1;
> + }
> +
> + spapr_cpu_init(spapr, cpu, errp);
> + if (*errp) {
> + return 1;
> + }
> +
> + spapr_cpu_reset(cpu);
should it be move to spapr_cpu_init() ?
> + return 0;
> +}
> +
> +static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> +{
> + sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + int spapr_max_cores = max_cpus / smp_threads;
this should be in machine code
> + Error *local_err = NULL;
> + int threads = 0;
> + int core_dt_id, core_id;
> + int smt = kvmppc_smt_threads();
> +
> + threads = object_property_get_int(OBJECT(dev), "threads", &local_err);
since spapr_core is inherited from cpu-core,
you can cast to cpu-core and use fields directly here instead of using
property setters.
> + if (local_err) {
> + goto out;
> + }
> +
> + if (threads != smp_threads) {
> + error_setg(&local_err, "threads must be %d", smp_threads);
> + goto out;
> + }
move to machine handler
> +
> + if (!core->oc) {
> + error_setg(&local_err, "cpu_model property isn't set");
> + goto out;
> + }
> +
> + core_dt_id = object_property_get_int(OBJECT(dev), "core", &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> + if (core_dt_id % smt) {
> + error_setg(&local_err, "invalid core id %d\n", core_dt_id);
> + goto out;
> + }
> +
> + core_id = core_dt_id / smt;
> + if (core_id < 0 || core_id >= spapr_max_cores) {
> + error_setg(&local_err, "core id %d out of range", core_dt_id);
> + goto out;
> + }
maybe due to nameing it's a bit confusing,
what's difference between core_id and core_dt_id?
> +
> + /*
> + * TODO: This check will be moved to ->pre_plug() as suggested by Igor
> + * when there is consensus pre_plug hook.
> + */
> + if (spapr->cores[core_id]) {
> + error_setg(&local_err, "core %d already populated", core_dt_id);
> + goto out;
> + }
> +
> + core->threads = g_new0(PowerPCCPU, threads);
> + spapr_cpu_core_create_threads(core, threads, &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> + object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child,
> &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> + return;
> +
> +out:
> + if (local_err) {
> + g_free(core->threads);
> + error_propagate(errp, local_err);
> + }
> +}
> +
> +static char *spapr_cpu_core_prop_get_cpu_model(Object *obj, Error **errp)
> +{
> + sPAPRCPUCore *core = SPAPR_CPU_CORE(obj);
> +
> + /*
> + * TODO: This returns the full type instead of just cpu_model. For eg,
> + * host-powerpc64-cpu is returned where just "host" is expected.
> + */
> + return g_strdup(object_class_get_name(core->oc));
> +}
> +
> +static void spapr_cpu_core_prop_set_cpu_model(Object *obj, const char *val,
> + Error **errp)
> +{
> + sPAPRCPUCore *core = SPAPR_CPU_CORE(obj);
> + MachineState *machine = MACHINE(qdev_get_machine());
> + ObjectClass *oc = cpu_class_by_name(TYPE_POWERPC_CPU, val);
> + ObjectClass *oc_base = cpu_class_by_name(TYPE_POWERPC_CPU,
> + machine->cpu_model);
> + if (!oc) {
> + error_setg(errp, "Unknown CPU model %s", val);
> + return;
> + }
> +
> + /*
> + * Currently cpu_model can't be different from what is specified with
> -cpu
> + */
> + if (strcmp(object_class_get_name(oc), object_class_get_name(oc_base))) {
> + error_setg(errp, "cpu_model must be %s", machine->cpu_model);
> + return;
> + }
> +
> + core->oc = oc;
> +}
> +
> +static void spapr_cpu_core_instance_init(Object *obj)
> +{
> + object_property_add_str(obj, "cpu_model",
> + spapr_cpu_core_prop_get_cpu_model,
> + spapr_cpu_core_prop_set_cpu_model,
> + NULL);
> +}
> +
> +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->realize = spapr_cpu_core_realize;
> +}
> +
> +static const TypeInfo spapr_cpu_core_type_info = {
> + .name = TYPE_SPAPR_CPU_CORE,
> + .parent = TYPE_CPU_CORE,
> + .instance_init = spapr_cpu_core_instance_init,
> + .instance_size = sizeof(sPAPRCPUCore),
> + .class_init = spapr_cpu_core_class_init,
> +};
> +
> +static void spapr_cpu_core_register_types(void)
> +{
> + type_register_static(&spapr_cpu_core_type_info);
> +}
> +
> +type_init(spapr_cpu_core_register_types)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 098d85d..c099c3c 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 */
> };
>
> @@ -79,6 +80,7 @@ struct sPAPRMachineState {
> /*< public >*/
> char *kvm_type;
> MemoryHotplugState hotplug_memory;
> + Object **cores;
I'd prefer "Object *cores[0];" as it tells us that it's an array
> };
>
> #define H_SUCCESS 0
> @@ -585,6 +587,8 @@ void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType
> drc_type,
> uint32_t count);
> void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
> uint32_t count);
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp);
> +void spapr_cpu_reset(void *opaque);
>
> /* rtas-configure-connector state */
> struct sPAPRConfigureConnectorState {
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> new file mode 100644
> index 0000000..48fb76a
> --- /dev/null
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -0,0 +1,28 @@
> +/*
> + * sPAPR CPU core device.
> + *
> + * Copyright (C) 2016 Bharata B Rao <bhar...@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_CORE_H
> +#define HW_SPAPR_CPU_CORE_H
> +
> +#include "hw/qdev.h"
> +#include "hw/cpu/core.h"
> +
> +#define TYPE_SPAPR_CPU_CORE "spapr-cpu-core"
> +#define SPAPR_CPU_CORE(obj) \
> + OBJECT_CHECK(sPAPRCPUCore, (obj), TYPE_SPAPR_CPU_CORE)
> +
> +typedef struct sPAPRCPUCore {
> + /*< private >*/
> + CPUCore parent_obj;
> +
> + /*< public >*/
> + ObjectClass *oc;
> + PowerPCCPU *threads;
> +} sPAPRCPUCore;
> +
> +#endif
