On Thu, Feb 09, 2017 at 12:08:37PM +0100, Igor Mammedov wrote: > Replace SPAPR specific cores[] array with generic > machine->possible_cpus and store core objects there. > It makes cores bookkeeping similar to x86 cpus and > will allow to unify similar code. > It would allow to replace cpu_index based NUMA node > mapping with iproperty based one (for -device created > cores) since possible_cpus carries board defined > topology/layout. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
Apart from one type noted below,
Acked-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> include/hw/ppc/spapr.h | 1 -
> hw/ppc/spapr.c | 129
> ++++++++++++++++++++++++++++++++++---------------
> 2 files changed, 90 insertions(+), 40 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index a2d8964..f9b17d8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -94,7 +94,6 @@ struct sPAPRMachineState {
> /*< public >*/
> char *kvm_type;
> MemoryHotplugState hotplug_memory;
> - Object **cores;
> };
>
> #define H_SUCCESS 0
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 37cb338..8d039ae 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1751,13 +1751,28 @@ static void spapr_validate_node_memory(MachineState
> *machine, Error **errp)
> }
> }
>
> +/* find cpu slot in machine->possible_cpus by core_id */
> +static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int
> *idx)
> +{
> + int index = id / smp_threads;
> +
> + if (index >= ms->possible_cpus->len) {
> + return NULL;
> + }
> + if (idx) {
> + *idx = index;
> + }
> + return &ms->possible_cpus->cpus[index];
> +}
> +
> static void spapr_init_cpus(sPAPRMachineState *spapr)
> {
> MachineState *machine = MACHINE(spapr);
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> char *type = spapr_get_cpu_core_type(machine->cpu_model);
> int smt = kvmppc_smt_threads();
> - int spapr_max_cores, spapr_cores;
> + const CPUArchIdList *possible_cpus;
> + int boot_cores_nr = smp_cpus / smp_threads;;
Stray extra ';' above.
> int i;
>
> if (!type) {
> @@ -1765,6 +1780,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> exit(1);
> }
>
> + possible_cpus = mc->possible_cpu_arch_ids(machine);
> if (mc->query_hotpluggable_cpus) {
> if (smp_cpus % smp_threads) {
> error_report("smp_cpus (%u) must be multiple of threads (%u)",
> @@ -1776,21 +1792,15 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> max_cpus, smp_threads);
> exit(1);
> }
> -
> - spapr_max_cores = max_cpus / smp_threads;
> - spapr_cores = smp_cpus / smp_threads;
> } else {
> if (max_cpus != smp_cpus) {
> error_report("This machine version does not support CPU
> hotplug");
> exit(1);
> }
> -
> - spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
> - spapr_cores = spapr_max_cores;
> + boot_cores_nr = possible_cpus->len;
> }
>
> - spapr->cores = g_new0(Object *, spapr_max_cores);
> - for (i = 0; i < spapr_max_cores; i++) {
> + for (i = 0; i < possible_cpus->len; i++) {
> int core_id = i * smp_threads;
>
> if (mc->query_hotpluggable_cpus) {
> @@ -1802,7 +1812,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> qemu_register_reset(spapr_drc_reset, drc);
> }
>
> - if (i < spapr_cores) {
> + if (i < boot_cores_nr) {
> Object *core = object_new(type);
> int nr_threads = smp_threads;
>
> @@ -2491,10 +2501,11 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int
> *fdt_offset,
> static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + MachineState *ms = MACHINE(qdev_get_machine());
> CPUCore *cc = CPU_CORE(dev);
> + CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
>
> - spapr->cores[cc->core_id / smp_threads] = NULL;
> + core_slot->cpu = NULL;
> object_unparent(OBJECT(dev));
> }
>
> @@ -2510,19 +2521,24 @@ static
> void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> - CPUCore *cc = CPU_CORE(dev);
> - int smt = kvmppc_smt_threads();
> - int index = cc->core_id / smp_threads;
> - sPAPRDRConnector *drc =
> - spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> + int index;
> + sPAPRDRConnector *drc;
> sPAPRDRConnectorClass *drck;
> Error *local_err = NULL;
> + CPUCore *cc = CPU_CORE(dev);
> + int smt = kvmppc_smt_threads();
>
> + if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
> + error_setg(errp, "Unable to find CPU core with core-id: %d",
> + cc->core_id);
> + return;
> + }
> if (index == 0) {
> error_setg(errp, "Boot CPU core may not be unplugged");
> return;
> }
>
> + drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> g_assert(drc);
>
> drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> @@ -2547,11 +2563,17 @@ static void spapr_core_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> Error *local_err = NULL;
> void *fdt = NULL;
> int fdt_offset = 0;
> - int index = cc->core_id / smp_threads;
> int smt = kvmppc_smt_threads();
> + CPUArchId *core_slot;
> + int index;
>
> + core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,
> &index);
> + if (!core_slot) {
> + error_setg(errp, "Unable to find CPU core with core-id: %d",
> + cc->core_id);
> + return;
> + }
> drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> - spapr->cores[index] = OBJECT(dev);
>
> g_assert(drc || !mc->query_hotpluggable_cpus);
>
> @@ -2568,7 +2590,6 @@ static void spapr_core_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged,
> &local_err);
> if (local_err) {
> g_free(fdt);
> - spapr->cores[index] = NULL;
> error_propagate(errp, local_err);
> return;
> }
> @@ -2590,6 +2611,7 @@ static void spapr_core_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> drck->set_isolation_state(drc,
> SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> }
> }
> + core_slot->cpu = OBJECT(dev);
> }
>
> static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> @@ -2597,13 +2619,12 @@ static void spapr_core_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> {
> MachineState *machine = MACHINE(OBJECT(hotplug_dev));
> MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
> - sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> - int spapr_max_cores = max_cpus / smp_threads;
> - int index;
> Error *local_err = NULL;
> CPUCore *cc = CPU_CORE(dev);
> char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
> const char *type = object_get_typename(OBJECT(dev));
> + CPUArchId *core_slot;
> + int index;
>
> if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
> error_setg(&local_err, "CPU hotplug not supported for this machine");
> @@ -2620,13 +2641,13 @@ static void spapr_core_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> goto out;
> }
>
> - index = cc->core_id / smp_threads;
> - if (index < 0 || index >= spapr_max_cores) {
> + core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,
> &index);
> + if (!core_slot) {
> error_setg(&local_err, "core id %d out of range", cc->core_id);
> goto out;
> }
>
> - if (spapr->cores[index]) {
> + if (core_slot->cpu) {
> error_setg(&local_err, "core %d already populated", cc->core_id);
> goto out;
> }
> @@ -2758,29 +2779,58 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned
> cpu_index)
> return cpu_index / smp_threads / smp_cores;
> }
>
> +static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState
> *machine)
> +{
> + int i;
> + int spapr_max_cores = max_cpus / smp_threads;
> + MachineClass *mc = MACHINE_GET_CLASS(machine);
> +
> + if (!mc->query_hotpluggable_cpus) {
> + spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
> + }
> + if (machine->possible_cpus) {
> + assert(machine->possible_cpus->len == spapr_max_cores);
> + return machine->possible_cpus;
> + }
> +
> + machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> + sizeof(CPUArchId) * spapr_max_cores);
> + machine->possible_cpus->len = spapr_max_cores;
> + for (i = 0; i < machine->possible_cpus->len; i++) {
> + int core_id = i * smp_threads;
> +
> + machine->possible_cpus->cpus[i].arch_id = core_id;
> + machine->possible_cpus->cpus[i].props.has_core_id = true;
> + machine->possible_cpus->cpus[i].props.core_id = core_id;
> + /* TODO: add 'has_node/node' here to describe
> + to which node core belongs */
> + }
> + return machine->possible_cpus;
> +}
> +
> static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState
> *machine)
> {
> int i;
> + Object *cpu;
> HotpluggableCPUList *head = NULL;
> - sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> - int spapr_max_cores = max_cpus / smp_threads;
> + const char *cpu_type;
>
> - for (i = 0; i < spapr_max_cores; i++) {
> + cpu = machine->possible_cpus->cpus[0].cpu;
> + assert(cpu); /* Boot cpu is always present */
> + cpu_type = object_get_typename(cpu);
> + for (i = 0; i < machine->possible_cpus->len; i++) {
> HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> - CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
>
> - cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
> - cpu_item->vcpus_count = smp_threads;
> - cpu_props->has_core_id = true;
> - cpu_props->core_id = i * smp_threads;
> - /* TODO: add 'has_node/node' here to describe
> - to which node core belongs */
> + cpu_item->type = g_strdup(cpu_type);
> + cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
> + cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> + sizeof(*cpu_item->props));
>
> - cpu_item->props = cpu_props;
> - if (spapr->cores[i]) {
> + cpu = machine->possible_cpus->cpus[i].cpu;
> + if (cpu) {
> cpu_item->has_qom_path = true;
> - cpu_item->qom_path = object_get_canonical_path(spapr->cores[i]);
> + cpu_item->qom_path = object_get_canonical_path(cpu);
> }
> list_item->value = cpu_item;
> list_item->next = head;
> @@ -2872,6 +2922,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> hc->plug = spapr_machine_device_plug;
> hc->unplug = spapr_machine_device_unplug;
> mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> + mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids;
> hc->unplug_request = spapr_machine_device_unplug_request;
>
> smc->dr_lmb_enabled = true;
--
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
signature.asc
Description: PGP signature
