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

[Igor Mammedov]

On Thu, 10 Mar 2016 10:42:26 +1100
David Gibson  wrote:

> On Wed, Mar 09, 2016 at 11:40:53AM +0100, Igor Mammedov wrote:
> > On Tue, 8 Mar 2016 15:26:27 +1100
> > David Gibson  wrote:  
> > > On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote:  
> > > > On Mon, 7 Mar 2016 14:36:55 +1100
> > > > David Gibson  wrote:  
> [snip]
> > > > > > > > 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.   
> > > > > > > >  
> > > > > > > 
> > > > > > > Ok that would suit us as there are two properties on which thread 
> > > > > > > creation
> > > > > > > is dependent upon: nr_threads and cpu_model. If thread objects 
> > > > > > > can be
> > > > > > > created at core realize time, then we don't have to resort to the 
> > > > > > > ugliness
> > > > > > > of creating the threads from either of the property setters. I 
> > > > > > > always
> > > > > > > assumed that we shouldn't be creating objects from realize, but 
> > > > > > > if that
> > > > > > > is fine, it is good.  
> > > > > > since realize is allowed to fail, it should be safe from hotplug pov
> > > > > > to create internal objects there, as far as proper cleanups are done
> > > > > > for failure path.  
> > > > > 
> > > > [...]
> > > > > I'm not clear from the above if you're also intending to move at least
> > > > > the adding of the threads as child properties is supposed to go into
> > > > > the base type,
> > > > I'm not sure that I've got question, could you please rephrase?
> > > 
> > > So, it seems like we're agreed that moving the nr_threads property to
> > > the base type is a good idea.
> > > 
> > > My question is, do we also move the object_property_add_child() calls
> > > for each thread to the base type (possibly via a helper function or
> > > method the base type provides to derived types)?  
> > I can't think of a reason to do so,
> > why can't subtype-core.realize() do it?  
> 
> It can, but I'm always suspicious of boilerplate stuff that every
> subtype *has* to do in order to work properly.
> 
> > What would one gain creating callbacks and calling them from base class?  
> 
> Enforcing - or at least making as easy as possible - consistency in
> the child object naming.
> 

I guess a patch would help to understand if it's worth of effort of adding
extra callbacks in base class.

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

[Igor Mammedov]

On Thu, 10 Mar 2016 16:04:29 +1100
David Gibson  wrote:

> On Wed, Mar 09, 2016 at 11:32:28AM +0100, Igor Mammedov wrote:
> > On Wed, 9 Mar 2016 13:55:51 +1100
> > David Gibson  wrote:
> >   
> > > On Tue, Mar 08, 2016 at 10:11:17AM +0100, Igor Mammedov wrote:  
> > > > On Tue, 8 Mar 2016 14:57:10 +1100
> > > > David Gibson  wrote:
> > > > 
> > > > > On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:
> > > > > > On Mon, 7 Mar 2016 14:01:55 +0530
> > > > > > Bharata B Rao  wrote:
> > > > > >   
> > > > > > > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote: 
> > > > > > >  
> > > > > > > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:  
> > > > > > > >   
> > > > > > > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > > > > > > Bharata B Rao  wrote:
> > > > > > > > > 
> > > > > > > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov 
> > > > > > > > > > wrote:
> > > > > > > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > > > > > > Bharata B Rao  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 
> > > > > > > > > > > > 
> > > > > > > > > > > > ---
> > > > > > > > > > > >  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 000..d8caf37
> > > > > > > > > > > > --- /dev/null
> > > > > > > > > > > > +++ b/hw/cpu/core.c
> > > > > > > > > > > > @@ -0,0 +1,44 @@
> > > > > > > > > > > > +/*
> > > > > > > > > > > > + * CPU core abstract device
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > > > > > 
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * 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,
> > > > > > > > > > > > +};
> > > > > > > > > > > > +
> > > > > > 

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

[David Gibson]

On Wed, Mar 09, 2016 at 11:32:28AM +0100, Igor Mammedov wrote:
> On Wed, 9 Mar 2016 13:55:51 +1100
> David Gibson  wrote:
> 
> > On Tue, Mar 08, 2016 at 10:11:17AM +0100, Igor Mammedov wrote:
> > > On Tue, 8 Mar 2016 14:57:10 +1100
> > > David Gibson  wrote:
> > >   
> > > > On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:  
> > > > > On Mon, 7 Mar 2016 14:01:55 +0530
> > > > > Bharata B Rao  wrote:
> > > > > 
> > > > > > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:
> > > > > > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > > > > > >   
> > > > > > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > > > > > Bharata B Rao  wrote:
> > > > > > > >   
> > > > > > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov 
> > > > > > > > > wrote:  
> > > > > > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > > > > > Bharata B Rao  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 
> > > > > > > > > > > ---
> > > > > > > > > > >  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 000..d8caf37
> > > > > > > > > > > --- /dev/null
> > > > > > > > > > > +++ b/hw/cpu/core.c
> > > > > > > > > > > @@ -0,0 +1,44 @@
> > > > > > > > > > > +/*
> > > > > > > > > > > + * CPU core abstract device
> > > > > > > > > > > + *
> > > > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > > > > 
> > > > > > > > > > > + *
> > > > > > > > > > > + * 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(_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
> > > > > > 

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

[David Gibson]

On Wed, Mar 09, 2016 at 11:40:53AM +0100, Igor Mammedov wrote:
> On Tue, 8 Mar 2016 15:26:27 +1100
> David Gibson  wrote:
> > On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote:
> > > On Mon, 7 Mar 2016 14:36:55 +1100
> > > David Gibson  wrote:
[snip]
> > > > > > > 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. 
> > > > > > >  
> > > > > > 
> > > > > > Ok that would suit us as there are two properties on which thread 
> > > > > > creation
> > > > > > is dependent upon: nr_threads and cpu_model. If thread objects can 
> > > > > > be
> > > > > > created at core realize time, then we don't have to resort to the 
> > > > > > ugliness
> > > > > > of creating the threads from either of the property setters. I 
> > > > > > always
> > > > > > assumed that we shouldn't be creating objects from realize, but if 
> > > > > > that
> > > > > > is fine, it is good.
> > > > > since realize is allowed to fail, it should be safe from hotplug pov
> > > > > to create internal objects there, as far as proper cleanups are done
> > > > > for failure path.
> > > >   
> > > [...]  
> > > > I'm not clear from the above if you're also intending to move at least
> > > > the adding of the threads as child properties is supposed to go into
> > > > the base type,  
> > > I'm not sure that I've got question, could you please rephrase?  
> > 
> > So, it seems like we're agreed that moving the nr_threads property to
> > the base type is a good idea.
> > 
> > My question is, do we also move the object_property_add_child() calls
> > for each thread to the base type (possibly via a helper function or
> > method the base type provides to derived types)?
> I can't think of a reason to do so,
> why can't subtype-core.realize() do it?

It can, but I'm always suspicious of boilerplate stuff that every
subtype *has* to do in order to work properly.

> What would one gain creating callbacks and calling them from base class?

Enforcing - or at least making as easy as possible - consistency in
the child object naming.

-- 
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

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

[Igor Mammedov]

On Tue, 8 Mar 2016 15:26:27 +1100
David Gibson  wrote:

> On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote:
> > On Mon, 7 Mar 2016 14:36:55 +1100
> > David Gibson  wrote:
> >   
> > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:  
> > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > Bharata B Rao  wrote:
> > > > 
> > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > Bharata B Rao  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 
> > > > > > > ---
> > > > > > >  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 000..d8caf37
> > > > > > > --- /dev/null
> > > > > > > +++ b/hw/cpu/core.c
> > > > > > > @@ -0,0 +1,44 @@
> > > > > > > +/*
> > > > > > > + * CPU core abstract device
> > > > > > > + *
> > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > + *
> > > > > > > + * 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(_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 000..2daa724
> > > > > > > --- /dev/null
> > > > > > > +++ b/include/hw/cpu/core.h
> > > > > > > @@ -0,0 +1,30 @@
> > > > > > > +/*
> > > > > > > + * CPU core abstract device
> > > > > > > + *
> > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > + *
> > > > > > > + * 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 

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

[Igor Mammedov]

On Wed, 9 Mar 2016 13:55:51 +1100
David Gibson  wrote:

> On Tue, Mar 08, 2016 at 10:11:17AM +0100, Igor Mammedov wrote:
> > On Tue, 8 Mar 2016 14:57:10 +1100
> > David Gibson  wrote:
> >   
> > > On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:  
> > > > On Mon, 7 Mar 2016 14:01:55 +0530
> > > > Bharata B Rao  wrote:
> > > > 
> > > > > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:
> > > > > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:  
> > > > > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > > > > Bharata B Rao  wrote:
> > > > > > >   
> > > > > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:  
> > > > > > > > 
> > > > > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > > > > Bharata B Rao  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 
> > > > > > > > > > ---
> > > > > > > > > >  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 000..d8caf37
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/hw/cpu/core.c
> > > > > > > > > > @@ -0,0 +1,44 @@
> > > > > > > > > > +/*
> > > > > > > > > > + * CPU core abstract device
> > > > > > > > > > + *
> > > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > > > 
> > > > > > > > > > + *
> > > > > > > > > > + * 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(_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 000..2daa724
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/include/hw/cpu/core.h
> > > > > > > > > > @@ -0,0 +1,30 @@
> > > > > > > > > > +/*
> > > > > > > > > > + * CPU core abstract device
> > > > > > > > > > + *
> > > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 

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

[David Gibson]

On Tue, Mar 08, 2016 at 10:11:17AM +0100, Igor Mammedov wrote:
> On Tue, 8 Mar 2016 14:57:10 +1100
> David Gibson  wrote:
> 
> > On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:
> > > On Mon, 7 Mar 2016 14:01:55 +0530
> > > Bharata B Rao  wrote:
> > >   
> > > > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:  
> > > > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > > > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > > > Bharata B Rao  wrote:
> > > > > > 
> > > > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > > > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > > > Bharata B Rao  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 
> > > > > > > > > ---
> > > > > > > > >  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 000..d8caf37
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/hw/cpu/core.c
> > > > > > > > > @@ -0,0 +1,44 @@
> > > > > > > > > +/*
> > > > > > > > > + * CPU core abstract device
> > > > > > > > > + *
> > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > > 
> > > > > > > > > + *
> > > > > > > > > + * 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(_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 000..2daa724
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/include/hw/cpu/core.h
> > > > > > > > > @@ -0,0 +1,30 @@
> > > > > > > > > +/*
> > > > > > > > > + * CPU core abstract device
> > > > > > > > > + *
> > > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > > 
> > > > > > > > > + *
> > > > > > > > > + * 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
> > > > 

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

[Igor Mammedov]

On Tue, 8 Mar 2016 14:57:10 +1100
David Gibson  wrote:

> On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:
> > On Mon, 7 Mar 2016 14:01:55 +0530
> > Bharata B Rao  wrote:
> >   
> > > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:  
> > > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > > Bharata B Rao  wrote:
> > > > > 
> > > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > > Bharata B Rao  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 
> > > > > > > > ---
> > > > > > > >  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 000..d8caf37
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/hw/cpu/core.c
> > > > > > > > @@ -0,0 +1,44 @@
> > > > > > > > +/*
> > > > > > > > + * CPU core abstract device
> > > > > > > > + *
> > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > 
> > > > > > > > + *
> > > > > > > > + * 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(_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 000..2daa724
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/include/hw/cpu/core.h
> > > > > > > > @@ -0,0 +1,30 @@
> > > > > > > > +/*
> > > > > > > > + * CPU core abstract device
> > > > > > > > + *
> > > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > > 
> > > > > > > > + *
> > > > > > > > + * 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) \
> > > > > > > > +

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

[David Gibson]

On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote:
> On Mon, 7 Mar 2016 14:36:55 +1100
> David Gibson  wrote:
> 
> > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > Bharata B Rao  wrote:
> > >   
> > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:  
> > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > Bharata B Rao  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 
> > > > > > ---
> > > > > >  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 000..d8caf37
> > > > > > --- /dev/null
> > > > > > +++ b/hw/cpu/core.c
> > > > > > @@ -0,0 +1,44 @@
> > > > > > +/*
> > > > > > + * CPU core abstract device
> > > > > > + *
> > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > + *
> > > > > > + * 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(_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 000..2daa724
> > > > > > --- /dev/null
> > > > > > +++ b/include/hw/cpu/core.h
> > > > > > @@ -0,0 +1,30 @@
> > > > > > +/*
> > > > > > + * CPU core abstract device
> > > > > > + *
> > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > + *
> > > > > > + * 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
> > > > 
> > > > Ok. Also note that this is a string property which is associated with 
> > > > the
> > > > link name (string) that we created from machine object to this core. I 
> > > > think
> > > > it would be ideal if 

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

[David Gibson]

On Mon, Mar 07, 2016 at 11:40:11AM +0100, Igor Mammedov wrote:
> On Mon, 7 Mar 2016 14:01:55 +0530
> Bharata B Rao  wrote:
> 
> > On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:
> > > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:  
> > > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > > Bharata B Rao  wrote:
> > > >   
> > > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:  
> > > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > > Bharata B Rao  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 
> > > > > > > ---
> > > > > > >  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 000..d8caf37
> > > > > > > --- /dev/null
> > > > > > > +++ b/hw/cpu/core.c
> > > > > > > @@ -0,0 +1,44 @@
> > > > > > > +/*
> > > > > > > + * CPU core abstract device
> > > > > > > + *
> > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > + *
> > > > > > > + * 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(_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 000..2daa724
> > > > > > > --- /dev/null
> > > > > > > +++ b/include/hw/cpu/core.h
> > > > > > > @@ -0,0 +1,30 @@
> > > > > > > +/*
> > > > > > > + * CPU core abstract device
> > > > > > > + *
> > > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > > + *
> > > > > > > + * 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 

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

[Igor Mammedov]

On Mon, 7 Mar 2016 14:01:55 +0530
Bharata B Rao  wrote:

> On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:
> > On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:  
> > > On Fri, 4 Mar 2016 16:32:53 +0530
> > > Bharata B Rao  wrote:
> > >   
> > > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:  
> > > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > > Bharata B Rao  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 
> > > > > > ---
> > > > > >  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 000..d8caf37
> > > > > > --- /dev/null
> > > > > > +++ b/hw/cpu/core.c
> > > > > > @@ -0,0 +1,44 @@
> > > > > > +/*
> > > > > > + * CPU core abstract device
> > > > > > + *
> > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > + *
> > > > > > + * 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(_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 000..2daa724
> > > > > > --- /dev/null
> > > > > > +++ b/include/hw/cpu/core.h
> > > > > > @@ -0,0 +1,30 @@
> > > > > > +/*
> > > > > > + * CPU core abstract device
> > > > > > + *
> > > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > > + *
> > > > > > + * 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
> > > > 
> > > > Ok. Also note that this is a string property which is associated with 
> > > > the
> > > > link name (string) that we created from machine object to this core. I 
> > > > think
> > > > it would be ideal if this 

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

[Igor Mammedov]

On Mon, 7 Mar 2016 14:36:55 +1100
David Gibson  wrote:

> On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > On Fri, 4 Mar 2016 16:32:53 +0530
> > Bharata B Rao  wrote:
> >   
> > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:  
> > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > Bharata B Rao  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 
> > > > > ---
> > > > >  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 000..d8caf37
> > > > > --- /dev/null
> > > > > +++ b/hw/cpu/core.c
> > > > > @@ -0,0 +1,44 @@
> > > > > +/*
> > > > > + * CPU core abstract device
> > > > > + *
> > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > + *
> > > > > + * 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(_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 000..2daa724
> > > > > --- /dev/null
> > > > > +++ b/include/hw/cpu/core.h
> > > > > @@ -0,0 +1,30 @@
> > > > > +/*
> > > > > + * CPU core abstract device
> > > > > + *
> > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > + *
> > > > > + * 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 
> > > >
> > > 
> > > Ok. Also note that this is a string property which is associated with the
> > > link name (string) that we created from machine object to this core. I 
> > > think
> > > it would be ideal if this becomes an interger  property in which case it
> > > becomes easier to feed the core location into your 
> > > CPUSlotProperties.core.  
> > agreed, it should be core number.  
> 
> The slot stuff is continuing to confuse me a bit.  I see that we need
> some kind of "address" value, but how best to do it is not clear to
> me.
> 
> Changing 

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

[Bharata B Rao]

On Mon, Mar 07, 2016 at 02:36:55PM +1100, David Gibson wrote:
> On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> > On Fri, 4 Mar 2016 16:32:53 +0530
> > Bharata B Rao  wrote:
> > 
> > > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > > Bharata B Rao  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 
> > > > > ---
> > > > >  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 000..d8caf37
> > > > > --- /dev/null
> > > > > +++ b/hw/cpu/core.c
> > > > > @@ -0,0 +1,44 @@
> > > > > +/*
> > > > > + * CPU core abstract device
> > > > > + *
> > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > + *
> > > > > + * 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(_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 000..2daa724
> > > > > --- /dev/null
> > > > > +++ b/include/hw/cpu/core.h
> > > > > @@ -0,0 +1,30 @@
> > > > > +/*
> > > > > + * CPU core abstract device
> > > > > + *
> > > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > > + *
> > > > > + * 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 
> > > >  
> > > 
> > > Ok. Also note that this is a string property which is associated with the
> > > link name (string) that we created from machine object to this core. I 
> > > think
> > > it would be ideal if this becomes an interger  property in which case it
> > > becomes easier to feed the core location into your CPUSlotProperties.core.
> > agreed, it should be core number.
> 
> The slot stuff is continuing to confuse me a bit.  I see that we need
> some kind of "address" value, but how best to do it is not clear to
> me.
> 
> Changing this to an integer sounds like it's 

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

[David Gibson]

On Fri, Mar 04, 2016 at 07:07:20PM +0100, Igor Mammedov wrote:
> On Fri, 4 Mar 2016 16:32:53 +0530
> Bharata B Rao  wrote:
> 
> > On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > > On Fri,  4 Mar 2016 12:24:16 +0530
> > > Bharata B Rao  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 
> > > > ---
> > > >  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 000..d8caf37
> > > > --- /dev/null
> > > > +++ b/hw/cpu/core.c
> > > > @@ -0,0 +1,44 @@
> > > > +/*
> > > > + * CPU core abstract device
> > > > + *
> > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > + *
> > > > + * 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(_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 000..2daa724
> > > > --- /dev/null
> > > > +++ b/include/hw/cpu/core.h
> > > > @@ -0,0 +1,30 @@
> > > > +/*
> > > > + * CPU core abstract device
> > > > + *
> > > > + * Copyright (C) 2016 Bharata B Rao 
> > > > + *
> > > > + * 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  
> > 
> > Ok. Also note that this is a string property which is associated with the
> > link name (string) that we created from machine object to this core. I think
> > it would be ideal if this becomes an interger  property in which case it
> > becomes easier to feed the core location into your CPUSlotProperties.core.
> agreed, it should be core number.

The slot stuff is continuing to confuse me a bit.  I see that we need
some kind of "address" value, but how best to do it is not clear to
me.

Changing this to an integer sounds like it's probably a good idea.
I'm a bit wary of just calling it "core" though.  Do all platforms
even necessarily have a core id?

I'm wondering if the addressing is something that needs to move the
the platform specific subtypes, while some other stuff can move to the
generic base type.

> > > on top of that I'd add numeric 'threads' property to base class so
> > 

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

[Igor Mammedov]

On Fri, 4 Mar 2016 16:32:53 +0530
Bharata B Rao  wrote:

> On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> > On Fri,  4 Mar 2016 12:24:16 +0530
> > Bharata B Rao  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 
> > > ---
> > >  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 000..d8caf37
> > > --- /dev/null
> > > +++ b/hw/cpu/core.c
> > > @@ -0,0 +1,44 @@
> > > +/*
> > > + * CPU core abstract device
> > > + *
> > > + * Copyright (C) 2016 Bharata B Rao 
> > > + *
> > > + * 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(_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 000..2daa724
> > > --- /dev/null
> > > +++ b/include/hw/cpu/core.h
> > > @@ -0,0 +1,30 @@
> > > +/*
> > > + * CPU core abstract device
> > > + *
> > > + * Copyright (C) 2016 Bharata B Rao 
> > > + *
> > > + * 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  
> 
> Ok. Also note that this is a string property which is associated with the
> link name (string) that we created from machine object to this core. I think
> it would be ideal if this becomes an interger  property in which case it
> becomes easier to feed the core location into your CPUSlotProperties.core.
agreed, it should be core number.


> > 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 

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

[Bharata B Rao]

On Fri, Mar 04, 2016 at 11:38:45AM +0100, Igor Mammedov wrote:
> On Fri,  4 Mar 2016 12:24:16 +0530
> Bharata B Rao  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 
> > ---
> >  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 000..d8caf37
> > --- /dev/null
> > +++ b/hw/cpu/core.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * CPU core abstract device
> > + *
> > + * Copyright (C) 2016 Bharata B Rao 
> > + *
> > + * 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(_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 000..2daa724
> > --- /dev/null
> > +++ b/include/hw/cpu/core.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * CPU core abstract device
> > + *
> > + * Copyright (C) 2016 Bharata B Rao 
> > + *
> > + * 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

Ok. Also note that this is a string property which is associated with the
link name (string) that we created from machine object to this core. I think
it would be ideal if this becomes an interger  property in which case it
becomes easier to feed the core location into your CPUSlotProperties.core.

> 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.

Ok that would suit us as there are two properties on which thread creation
is dependent upon: nr_threads and cpu_model. If thread objects can be
created at core realize time, then we don't have to resort to the ugliness
of creating the threads from either of the property setters. I always
assumed that we shouldn't be creating objects from realize, but if that
is 

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

[Igor Mammedov]

On Fri,  4 Mar 2016 12:24:16 +0530
Bharata B Rao  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 
> ---
>  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 000..d8caf37
> --- /dev/null
> +++ b/hw/cpu/core.c
> @@ -0,0 +1,44 @@
> +/*
> + * CPU core abstract device
> + *
> + * Copyright (C) 2016 Bharata B Rao 
> + *
> + * 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(_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 000..2daa724
> --- /dev/null
> +++ b/include/hw/cpu/core.h
> @@ -0,0 +1,30 @@
> +/*
> + * CPU core abstract device
> + *
> + * Copyright (C) 2016 Bharata B Rao 
> + *
> + * 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