| Message ID | 1459413561-30745-10-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 31, 2016 at 02:09:18PM +0530, Bharata B Rao wrote: > 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 <bharata@linux.vnet.ibm.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > 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) { > + 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); > + } > } > > 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
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 <bharata@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 >
On Fri, Apr 08, 2016 at 06:35:19PM -0500, Michael Roth wrote: > 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 <bharata@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. In the next patch, I create DR connectors for all spapr_max_cores but create only spapr_cores number of cores (boot time cores). Having said that I will have the for loop changed to spapr_cores in this patch and move to spapr_max_cores in the next patch. > > > + 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? CPU core devices are created only when CPU DR capability is present. To support migration of older guests that don't know about CPU core devices, I retain the CPU thread initialization code as above. Regards, Bharata.
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) { + 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); + } } 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
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 <bharata@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(-)