Quoting Bharata B Rao (2016-03-31 03:39:18) > 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/spapr.c | 73 > +++++++++++++++++++++++++++++++++++------ > hw/ppc/spapr_cpu_core.c | 45 +++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 2 ++ > include/hw/ppc/spapr_cpu_core.h | 3 ++ > 4 files changed, 113 insertions(+), 10 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 45ac5dc..1ead043 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> > > @@ -1614,6 +1615,10 @@ static void spapr_boot_set(void *opaque, const char > *boot_device, > machine->boot_order = g_strdup(boot_device); > } > > +/* > + * 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; > @@ -1644,6 +1649,7 @@ void spapr_cpu_init(sPAPRMachineState *spapr, > PowerPCCPU *cpu, Error **errp) > xics_cpu_setup(spapr->icp, cpu); > > qemu_register_reset(spapr_cpu_reset, cpu); > + spapr_cpu_reset(cpu); > } > > /* > @@ -1727,7 +1733,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(); > @@ -1741,6 +1746,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; > > @@ -1787,8 +1808,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), > XICS_IRQS, &error_fatal); > > if (smc->dr_lmb_enabled) { > @@ -1799,13 +1819,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) {
Is there any reason to not just have the for() loop stop at spapr_cores? Maybe I missed something in the subsequent patches, but it seems like we never end up doing anything beyond i < spapr_cores. > + char *type = spapr_get_cpu_core_type(machine->cpu_model); > + Object *core = object_new(type); > + > + g_free(type); > + 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); Is there anything that prevents us using core modeling in the non-DR case as well? Having management of invididual threads/spapr_cpu_init contained in within one place seems beneficial beyond hotplug. Is it because cores are currently limited to cpus == "threads" via pre_plug hook? Maybe it makes sense to loosen that restriction in the special case where DR isn't enabled? > + } > } > > if (kvm_enabled()) { > @@ -2257,10 +2298,19 @@ static void > spapr_machine_device_unplug(HotplugHandler *hotplug_dev, > } > } > > +static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { > + spapr_core_pre_plug(hotplug_dev, dev, errp); > + } > +} > + > 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; > @@ -2299,11 +2349,13 @@ 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; > + hc->pre_plug = spapr_machine_device_pre_plug; > hc->plug = spapr_machine_device_plug; > hc->unplug = spapr_machine_device_unplug; > 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; > } > @@ -2383,6 +2435,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 > index 3751a54..640d143 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -15,6 +15,40 @@ > #include <sysemu/cpus.h> > #include "target-ppc/kvm_ppc.h" > > +void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); > + int spapr_max_cores = max_cpus / smp_threads; > + int index; > + int smt = kvmppc_smt_threads(); > + Error *local_err = NULL; > + CPUCore *cc = CPU_CORE(dev); > + > + if (cc->threads != smp_threads) { > + error_setg(&local_err, "threads must be %d", smp_threads); > + goto out; > + } > + > + if (cc->core % smt) { > + error_setg(&local_err, "invalid core id %d\n", cc->core); > + goto out; > + } > + > + index = cc->core / smt; > + if (index < 0 || index >= spapr_max_cores) { > + error_setg(&local_err, "core id %d out of range", cc->core); > + goto out; > + } > + > + if (spapr->cores[index]) { > + error_setg(&local_err, "core %d already populated", cc->core); > + } > + > +out: > + error_propagate(errp, local_err); > +} > + > static const TypeInfo spapr_cpu_core_type_info = { > .name = TYPE_SPAPR_CPU_CORE, > .parent = TYPE_CPU_CORE, > @@ -146,3 +180,14 @@ static void spapr_cpu_core_register_types(void) > } > > type_init(spapr_cpu_core_register_types) > + > +/* > + * TODO: Looks fragile :( > + */ > +char *spapr_get_cpu_core_type(const char *model) > +{ > + char core_type[32]; > + > + snprintf(core_type, 32, "%s-%s", model, TYPE_SPAPR_CPU_CORE); > + return g_strdup(core_type); > +} > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 0fdf448..a6956c0 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; > }; > > #define H_SUCCESS 0 > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > index 71e69c0..f08f291 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -60,4 +60,7 @@ typedef struct POWER8sPAPRCPUCore { > ObjectClass *cpu; > } POWER8sPAPRCPUCore; > > +void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp); > +char *spapr_get_cpu_core_type(const char *model); > #endif > -- > 2.1.0 >