Re: [Qemu-devel] [RFC PATCH v1 05/10] cpu: Abstract CPU core type

[Igor Mammedov <imammedo@redhat.com>]

On Fri,  4 Mar 2016 12:24:16 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> Add an abstract CPU core type that could be used by machines that want
> to define and hotplug CPUs in core granularity.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  hw/cpu/Makefile.objs  |  1 +
>  hw/cpu/core.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/cpu/core.h | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+)
>  create mode 100644 hw/cpu/core.c
>  create mode 100644 include/hw/cpu/core.h
> 
> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
> index 0954a18..942a4bb 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 += core.o
>  
> diff --git a/hw/cpu/core.c b/hw/cpu/core.c
> new file mode 100644
> index 0000000..d8caf37
> --- /dev/null
> +++ b/hw/cpu/core.c
> @@ -0,0 +1,44 @@
> +/*
> + * CPU core abstract 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/core.h"
> +
> +static char *core_prop_get_slot(Object *obj, Error **errp)
> +{
> +    CPUCore *core = CPU_CORE(obj);
> +
> +    return g_strdup(core->slot);
> +}
> +
> +static void core_prop_set_slot(Object *obj, const char *val, Error **errp)
> +{
> +    CPUCore *core = CPU_CORE(obj);
> +
> +    core->slot = g_strdup(val);
> +}
> +
> +static void cpu_core_instance_init(Object *obj)
> +{
> +    object_property_add_str(obj, "slot", core_prop_get_slot, core_prop_set_slot,
> +                            NULL);
> +}
> +
> +static const TypeInfo cpu_core_type_info = {
> +    .name = TYPE_CPU_CORE,
> +    .parent = TYPE_DEVICE,
> +    .abstract = true,
> +    .instance_size = sizeof(CPUCore),
> +    .instance_init = cpu_core_instance_init,
> +};
> +
> +static void cpu_core_register_types(void)
> +{
> +    type_register_static(&cpu_core_type_info);
> +}
> +
> +type_init(cpu_core_register_types)
> diff --git a/include/hw/cpu/core.h b/include/hw/cpu/core.h
> new file mode 100644
> index 0000000..2daa724
> --- /dev/null
> +++ b/include/hw/cpu/core.h
> @@ -0,0 +1,30 @@
> +/*
> + * CPU core abstract 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_CORE_H
> +#define HW_CPU_CORE_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev.h"
> +
> +#define TYPE_CPU_CORE "cpu-core"
> +
> +#define CPU_CORE(obj) \
> +    OBJECT_CHECK(CPUCore, (obj), TYPE_CPU_CORE)
> +
> +typedef struct CPUCore {
> +    /*< private >*/
> +    DeviceState parent_obj;
> +
> +    /*< public >*/
> +    char *slot;
> +} CPUCore;
> +
> +#define CPU_CORE_SLOT_PROP "slot"
as it's generic property I'd rename to 'core' so it would fit all users


on top of that I'd add numeric 'threads' property to base class so
all derived cores would inherit it.

Then as easy integration with -smp threads=x, a machine could push
a global variable 'cpu-core.threads=[smp_threads]' which would
make every created cpu-core object to have threads set
at instance_init() time (device_init).

That way user won't have to specify 'threads=y' for every
  device_add spapr-core,core=x
as it will be taken from global property 'cpu-core.threads'
but if user wishes he/she still could override global by explicitly
providing thread property at device_add time:
  device_add spapr-core,core=x,threads=y

wrt this series it would mean, instead of creating threads in property
setter, delaying threads creation to core.realize() time,
but since realize is allowed to fail it should be fine do so.

> +
> +#endif