Patchwork [8/8] qom: Make CPU a child of DeviceState

login
register
mail settings
Submitter Eduardo Habkost
Date Dec. 5, 2012, 4:49 p.m.
Message ID <1354726153-30264-9-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/203902/
State New
Headers show

Comments

Eduardo Habkost - Dec. 5, 2012, 4:49 p.m.
This finally makes the CPU class a child of DeviceState, allowing us to
start using DeviceState properties on CPU subclasses.

It has no_user=1, as creating CPUs using -device doesn't work yet.

(based on a previous patch from Igor Mammedov)

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 (imammedo) -> v2 (ehabkost):
 - Change CPU type declaration to hae TYPE_DEVICE as parent

Changes v2 -> v3 (ehabkost):
 - Set no_user=1 on the CPU class
---
 include/qemu/cpu.h | 6 +++---
 qom/cpu.c          | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)
Andreas Färber - Dec. 12, 2012, 1:34 p.m.
Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> This finally makes the CPU class a child of DeviceState, allowing us to
> start using DeviceState properties on CPU subclasses.
> 
> It has no_user=1, as creating CPUs using -device doesn't work yet.
> 
> (based on a previous patch from Igor Mammedov)
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 (imammedo) -> v2 (ehabkost):
>  - Change CPU type declaration to hae TYPE_DEVICE as parent
> 
> Changes v2 -> v3 (ehabkost):
>  - Set no_user=1 on the CPU class
> ---
>  include/qemu/cpu.h | 6 +++---
>  qom/cpu.c          | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index 61b7698..bc004fd 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -20,7 +20,7 @@
>  #ifndef QEMU_CPU_H
>  #define QEMU_CPU_H
>  
> -#include "qemu/object.h"
> +#include "hw/qdev-core.h"
>  #include "qemu-thread.h"
>  
>  /**
> @@ -46,7 +46,7 @@ typedef struct CPUState CPUState;
>   */
>  typedef struct CPUClass {
>      /*< private >*/
> -    ObjectClass parent_class;
> +    DeviceClass parent_class;
>      /*< public >*/
>  
>      void (*reset)(CPUState *cpu);
> @@ -62,7 +62,7 @@ typedef struct CPUClass {
>   */
>  struct CPUState {
>      /*< private >*/
> -    Object parent_obj;
> +    DeviceState parent_obj;
>      /*< public >*/
>  
>      struct QemuThread *thread;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5b36046..d301f72 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-core.h"
>  
>  void cpu_reset(CPUState *cpu)
>  {
> @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
>  
>  static void cpu_class_init(ObjectClass *klass, void *data)
>  {
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>      CPUClass *k = CPU_CLASS(klass);
>  
>      k->reset = cpu_common_reset;
> +    dc->no_user = 1;
>  }
>  
>  static TypeInfo cpu_type_info = {
>      .name = TYPE_CPU,
> -    .parent = TYPE_OBJECT,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(CPUState),
>      .abstract = true,
>      .class_size = sizeof(CPUClass),

This patch makes the CPU a device and looks good so far but does not
initialize the devices in cpu_*_init() like Anthony did in his previous
patch. I am unsure whether you forgot to do so or whether you wanted to
help keep our new CPU code clean of old-style qdev_init_nofail() calls?
Since you don't add a qdev initfn here the main difference will be the
devices internally staying in "created" rather than "initialized" state.

If we merge some patch that adds the "realized" property first (probably
not through qom-cpu tree then) we could avoid qdev_init*() but the end
result targetted by Paolo was not to have object creators worry about
realization at all through recursive realization. So either solution
needs to be changed later on.

Andreas
Eduardo Habkost - Dec. 12, 2012, 1:59 p.m.
On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
[...]
> >  static TypeInfo cpu_type_info = {
> >      .name = TYPE_CPU,
> > -    .parent = TYPE_OBJECT,
> > +    .parent = TYPE_DEVICE,
> >      .instance_size = sizeof(CPUState),
> >      .abstract = true,
> >      .class_size = sizeof(CPUClass),
> 
> This patch makes the CPU a device and looks good so far but does not
> initialize the devices in cpu_*_init() like Anthony did in his previous
> patch. I am unsure whether you forgot to do so or whether you wanted to
> help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> Since you don't add a qdev initfn here the main difference will be the
> devices internally staying in "created" rather than "initialized" state.

I think I used a version without the qdev_init_nofail() as base for this
series (we had multiple proposals being sent in parallel, in the
beginning), and in the end I forgot that we had a version with those
calls being added.

The CPU classes don't set any DeviceClass.init() method, so in theory
the missing qdev_init() calls wouldn't make any difference by now. On
the other hand, keeping the device in "created" state sounds bad... but
maybe this acceptable while we are still converting the CPU realize()
functions to fit inside the DeviceState initialization abstraction?


> 
> If we merge some patch that adds the "realized" property first (probably
> not through qom-cpu tree then) we could avoid qdev_init*() but the end
> result targetted by Paolo was not to have object creators worry about
> realization at all through recursive realization. So either solution
> needs to be changed later on.

I am interested in this series mainly because of the other features
provided by DeviceState: the static property and global-properties
system. The initialization abstraction provided by DeviceState will be
useful as well, but I also don't know what will be the best approach to
finally make the cpu_*init() functions sane. I still need to take a
better look at your QOM realize() RFC.
Igor Mammedov - Dec. 12, 2012, 2:27 p.m.
On Wed, 12 Dec 2012 11:59:01 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> [...]
> > >  static TypeInfo cpu_type_info = {
> > >      .name = TYPE_CPU,
> > > -    .parent = TYPE_OBJECT,
> > > +    .parent = TYPE_DEVICE,
> > >      .instance_size = sizeof(CPUState),
> > >      .abstract = true,
> > >      .class_size = sizeof(CPUClass),
> > 
> > This patch makes the CPU a device and looks good so far but does not
> > initialize the devices in cpu_*_init() like Anthony did in his previous
> > patch. I am unsure whether you forgot to do so or whether you wanted to
> > help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> > Since you don't add a qdev initfn here the main difference will be the
> > devices internally staying in "created" rather than "initialized" state.
> 
> I think I used a version without the qdev_init_nofail() as base for this
> series (we had multiple proposals being sent in parallel, in the
> beginning), and in the end I forgot that we had a version with those
> calls being added.
> 
> The CPU classes don't set any DeviceClass.init() method, so in theory
> the missing qdev_init() calls wouldn't make any difference by now. On
> the other hand, keeping the device in "created" state sounds bad... but
> maybe this acceptable while we are still converting the CPU realize()
> functions to fit inside the DeviceState initialization abstraction?
Testing shows that lack of qdev_create()/init() doesn't break anything so
far. I was planing to send hot-plug RFC after properties and subclasses for
x86 are done and temporally wrap x86_cpu_realize() inside DeviceClass.init()
so we could use qdev_create()/init() and device_add() for CPU.
It's still on my TODO list. Perhaps after I resubmit properties series (I
hope to do it this week), I'll redo hot-plug prototype using as a base my
experimental subclasses branch
https://github.com/imammedo/qemu/commits/x86-cpu-classes.WIP
Eduardo Habkost - Dec. 12, 2012, 2:36 p.m.
On Wed, Dec 12, 2012 at 03:27:24PM +0100, Igor Mammedov wrote:
> On Wed, 12 Dec 2012 11:59:01 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> > > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > [...]
> > > >  static TypeInfo cpu_type_info = {
> > > >      .name = TYPE_CPU,
> > > > -    .parent = TYPE_OBJECT,
> > > > +    .parent = TYPE_DEVICE,
> > > >      .instance_size = sizeof(CPUState),
> > > >      .abstract = true,
> > > >      .class_size = sizeof(CPUClass),
> > > 
> > > This patch makes the CPU a device and looks good so far but does not
> > > initialize the devices in cpu_*_init() like Anthony did in his previous
> > > patch. I am unsure whether you forgot to do so or whether you wanted to
> > > help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> > > Since you don't add a qdev initfn here the main difference will be the
> > > devices internally staying in "created" rather than "initialized" state.
> > 
> > I think I used a version without the qdev_init_nofail() as base for this
> > series (we had multiple proposals being sent in parallel, in the
> > beginning), and in the end I forgot that we had a version with those
> > calls being added.
> > 
> > The CPU classes don't set any DeviceClass.init() method, so in theory
> > the missing qdev_init() calls wouldn't make any difference by now. On
> > the other hand, keeping the device in "created" state sounds bad... but
> > maybe this acceptable while we are still converting the CPU realize()
> > functions to fit inside the DeviceState initialization abstraction?
> Testing shows that lack of qdev_create()/init() doesn't break anything so
> far. I was planing to send hot-plug RFC after properties and subclasses for
> x86 are done and temporally wrap x86_cpu_realize() inside DeviceClass.init()
> so we could use qdev_create()/init() and device_add() for CPU.

It seems to be easy to gradually replace the existing cpu_*_realize()
calls inside cpu_*_init() with qdev_init[_nofail]() calls (making
DeviceClas.init point to cpu_*_realize()), one architecture at a time.

I mean: one day we should kill the cpu_*_init() functions and make it
CPU creation/initialization generic code. But while we don't do that, a
simple cpu_*_realize()->qdev_init() replacement sounds trivial.

But first I want to understand the difference between DeviceClass.init()
and the proposed DeviceClass.realize() abstraction (they look exactly
the same to me). I just sent a question as reply to Andreas' realizefn
RFC.

> It's still on my TODO list. Perhaps after I resubmit properties series (I
> hope to do it this week), I'll redo hot-plug prototype using as a base my
> experimental subclasses branch
> https://github.com/imammedo/qemu/commits/x86-cpu-classes.WIP
Andreas Färber - Dec. 14, 2012, 3:29 p.m.
Am 12.12.2012 15:27, schrieb Igor Mammedov:
> On Wed, 12 Dec 2012 11:59:01 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>> On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
>>> This patch makes the CPU a device and looks good so far but does not
>>> initialize the devices in cpu_*_init() like Anthony did in his previous
>>> patch. I am unsure whether you forgot to do so or whether you wanted to
>>> help keep our new CPU code clean of old-style qdev_init_nofail() calls?
>>> Since you don't add a qdev initfn here the main difference will be the
>>> devices internally staying in "created" rather than "initialized" state.
>>
>> I think I used a version without the qdev_init_nofail() as base for this
>> series (we had multiple proposals being sent in parallel, in the
>> beginning), and in the end I forgot that we had a version with those
>> calls being added.
>>
>> The CPU classes don't set any DeviceClass.init() method, so in theory
>> the missing qdev_init() calls wouldn't make any difference by now. On
>> the other hand, keeping the device in "created" state sounds bad... but
>> maybe this acceptable while we are still converting the CPU realize()
>> functions to fit inside the DeviceState initialization abstraction?
> Testing shows that lack of qdev_create()/init() doesn't break anything so
> far.

The latest motivation for making the CPU a device was to have the static
properties infrastructure for machine/CPU versioning. The global
property defaults are set in qdev's instance_init, so object_new() seems
fine for that.

qdev_[try_]create() would further set the parent bus to SysBus if NULL.
The CPU is not a SysBusDevice so I think not using qdev_create() may be
safer... Maybe Anthony or Paolo can confirm?

Andreas
Paolo Bonzini - Dec. 14, 2012, 3:40 p.m.
Il 14/12/2012 16:29, Andreas Färber ha scritto:
> The latest motivation for making the CPU a device was to have the static
> properties infrastructure for machine/CPU versioning. The global
> property defaults are set in qdev's instance_init, so object_new() seems
> fine for that.
> 
> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
> The CPU is not a SysBusDevice so I think not using qdev_create() may be
> safer... Maybe Anthony or Paolo can confirm?

I think various parts of qdev assume there is a bus, so actually using
SysBus would be safer (though uglier).

Paolo
Andreas Färber - Dec. 14, 2012, 3:44 p.m.
Am 14.12.2012 16:40, schrieb Paolo Bonzini:
> Il 14/12/2012 16:29, Andreas Färber ha scritto:
>> The latest motivation for making the CPU a device was to have the static
>> properties infrastructure for machine/CPU versioning. The global
>> property defaults are set in qdev's instance_init, so object_new() seems
>> fine for that.
>>
>> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
>> The CPU is not a SysBusDevice so I think not using qdev_create() may be
>> safer... Maybe Anthony or Paolo can confirm?
> 
> I think various parts of qdev assume there is a bus, so actually using
> SysBus would be safer (though uglier).

Hm, Anthony told me with one of his qbus refactoring patches back in
qom-next the last remaining assumptions (info qdm) were removed...

Probably we're the first to test though. ;)

Andreas
Eduardo Habkost - Dec. 14, 2012, 5:56 p.m.
On Fri, Dec 14, 2012 at 04:44:24PM +0100, Andreas Färber wrote:
> Am 14.12.2012 16:40, schrieb Paolo Bonzini:
> > Il 14/12/2012 16:29, Andreas Färber ha scritto:
> >> The latest motivation for making the CPU a device was to have the static
> >> properties infrastructure for machine/CPU versioning. The global
> >> property defaults are set in qdev's instance_init, so object_new() seems
> >> fine for that.
> >>
> >> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
> >> The CPU is not a SysBusDevice so I think not using qdev_create() may be
> >> safer... Maybe Anthony or Paolo can confirm?
> > 
> > I think various parts of qdev assume there is a bus, so actually using
> > SysBus would be safer (though uglier).
> 
> Hm, Anthony told me with one of his qbus refactoring patches back in
> qom-next the last remaining assumptions (info qdm) were removed...
> 
> Probably we're the first to test though. ;)

BTW, we're also not including SysBus, on *-user.
Andreas Färber - Jan. 2, 2013, 3:08 p.m.
Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> This finally makes the CPU class a child of DeviceState, allowing us to
> start using DeviceState properties on CPU subclasses.

To avoid confusion with child<> properties and DeviceState vs.
DeviceClass I have reworded this to "subclass of Device" in my
qom-cpu-dev queue.

> 
> It has no_user=1, as creating CPUs using -device doesn't work yet.
> 

> (based on a previous patch from Igor Mammedov)

Can this comment be turned into or amended by the usual Signed-off-by?

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 (imammedo) -> v2 (ehabkost):
>  - Change CPU type declaration to hae TYPE_DEVICE as parent
> 
> Changes v2 -> v3 (ehabkost):
>  - Set no_user=1 on the CPU class
> ---
>  include/qemu/cpu.h | 6 +++---
>  qom/cpu.c          | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index 61b7698..bc004fd 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -20,7 +20,7 @@
>  #ifndef QEMU_CPU_H
>  #define QEMU_CPU_H
>  
> -#include "qemu/object.h"
> +#include "hw/qdev-core.h"
>  #include "qemu-thread.h"
>  
>  /**
[...]
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5b36046..d301f72 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-core.h"

Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.

>  
>  void cpu_reset(CPUState *cpu)
>  {
> @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
>  
>  static void cpu_class_init(ObjectClass *klass, void *data)
>  {
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>      CPUClass *k = CPU_CLASS(klass);
>  
>      k->reset = cpu_common_reset;
> +    dc->no_user = 1;
>  }

I wonder if we should add a comment that we are intentionally not
hooking up dc->reset (yet)?

>  
>  static TypeInfo cpu_type_info = {

Would like to add the missing const while touching this.

>      .name = TYPE_CPU,
> -    .parent = TYPE_OBJECT,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(CPUState),
>      .abstract = true,
>      .class_size = sizeof(CPUClass),

My testing so far confirms that the combination of object_new() without
qdev_init[_nofail]() is working fine.

Using qdev_create() in the current state of stubs would lead to a silly
if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
expect qdev_create() to grow in functionality, so continuing to use
object_new() should be okay - SoCs like my Tegra model may want to use
object_initialize() so we cannot prescribe using qdev_create() anyway.

qdev_init_nofail() would call the qdev initfn (to be replaced by
realizefn, not used for CPU in this patch), then if no parent add it to
/machine/unassigned, register VMSD if not NULL, update the internal
state (blocking static property changes) and if hotplugged reset (unused
due to dc->no_user and lack of dc->reset). The /machine/unassigned part
may be interesting, e.g., for APIC modelling (so that we can model the
former ptr property / now pointer-setting as a link<> property).

With these considerations I am leaning towards accepting this patch if
nobody objects, so that we can move on to the next refactorings...

Regards,
Andreas
Igor Mammedov - Jan. 2, 2013, 4:40 p.m.
On Wed, 02 Jan 2013 16:08:42 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > This finally makes the CPU class a child of DeviceState, allowing us to
> > start using DeviceState properties on CPU subclasses.
> 
> To avoid confusion with child<> properties and DeviceState vs.
> DeviceClass I have reworded this to "subclass of Device" in my
> qom-cpu-dev queue.
> 
> > 
> > It has no_user=1, as creating CPUs using -device doesn't work yet.
> > 
> 
> > (based on a previous patch from Igor Mammedov)
> 
> Can this comment be turned into or amended by the usual Signed-off-by?
Signed-off-by should be ok.

> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 (imammedo) -> v2 (ehabkost):
> >  - Change CPU type declaration to hae TYPE_DEVICE as parent
> > 
> > Changes v2 -> v3 (ehabkost):
> >  - Set no_user=1 on the CPU class
> > ---
> >  include/qemu/cpu.h | 6 +++---
> >  qom/cpu.c          | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> > index 61b7698..bc004fd 100644
> > --- a/include/qemu/cpu.h
> > +++ b/include/qemu/cpu.h
> > @@ -20,7 +20,7 @@
> >  #ifndef QEMU_CPU_H
> >  #define QEMU_CPU_H
> >  
> > -#include "qemu/object.h"
> > +#include "hw/qdev-core.h"
> >  #include "qemu-thread.h"
> >  
> >  /**
> [...]
> > diff --git a/qom/cpu.c b/qom/cpu.c
> > index 5b36046..d301f72 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include "qemu/cpu.h"
> >  #include "qemu-common.h"
> > +#include "hw/qdev-core.h"
> 
> Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.
> 
> >  
> >  void cpu_reset(CPUState *cpu)
> >  {
> > @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
> >  
> >  static void cpu_class_init(ObjectClass *klass, void *data)
> >  {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> >      CPUClass *k = CPU_CLASS(klass);
> >  
> >      k->reset = cpu_common_reset;
> > +    dc->no_user = 1;
> >  }
> 
> I wonder if we should add a comment that we are intentionally not
> hooking up dc->reset (yet)?
not relevant to this patch, could be separate patch though.

> 
> >  
> >  static TypeInfo cpu_type_info = {
> 
> Would like to add the missing const while touching this.
> 
> >      .name = TYPE_CPU,
> > -    .parent = TYPE_OBJECT,
> > +    .parent = TYPE_DEVICE,
> >      .instance_size = sizeof(CPUState),
> >      .abstract = true,
> >      .class_size = sizeof(CPUClass),
> 
> My testing so far confirms that the combination of object_new() without
> qdev_init[_nofail]() is working fine.
+1, I tested this combo for (x86)-(softmmu|linux-user) targets, no issues were
found so far.

> 
> Using qdev_create() in the current state of stubs would lead to a silly
> if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
> expect qdev_create() to grow in functionality, so continuing to use
> object_new() should be okay - SoCs like my Tegra model may want to use
> object_initialize() so we cannot prescribe using qdev_create() anyway.
> 
> qdev_init_nofail() would call the qdev initfn (to be replaced by
> realizefn, not used for CPU in this patch), then if no parent add it to
> /machine/unassigned, register VMSD if not NULL, update the internal
> state (blocking static property changes) and if hotplugged reset (unused
> due to dc->no_user and lack of dc->reset). The /machine/unassigned part
> may be interesting, e.g., for APIC modelling (so that we can model the
> former ptr property / now pointer-setting as a link<> property).
> 
> With these considerations I am leaning towards accepting this patch if
> nobody objects, so that we can move on to the next refactorings...
+1

> 
> Regards,
> Andreas
>
Eduardo Habkost - Jan. 2, 2013, 4:49 p.m.
On Wed, Jan 02, 2013 at 05:40:58PM +0100, Igor Mammedov wrote:
> On Wed, 02 Jan 2013 16:08:42 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > > This finally makes the CPU class a child of DeviceState, allowing us to
> > > start using DeviceState properties on CPU subclasses.
> > 
> > To avoid confusion with child<> properties and DeviceState vs.
> > DeviceClass I have reworded this to "subclass of Device" in my
> > qom-cpu-dev queue.
> > 
> > > 
> > > It has no_user=1, as creating CPUs using -device doesn't work yet.
> > > 
> > 
> > > (based on a previous patch from Igor Mammedov)
> > 
> > Can this comment be turned into or amended by the usual Signed-off-by?
> Signed-off-by should be ok.

OK to me, as well. Should I resubmit, or can Andreas edit it when
committing the patch?

> 
> > 
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Changes v1 (imammedo) -> v2 (ehabkost):
> > >  - Change CPU type declaration to hae TYPE_DEVICE as parent
> > > 
> > > Changes v2 -> v3 (ehabkost):
> > >  - Set no_user=1 on the CPU class
> > > ---
> > >  include/qemu/cpu.h | 6 +++---
> > >  qom/cpu.c          | 5 ++++-
> > >  2 files changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> > > index 61b7698..bc004fd 100644
> > > --- a/include/qemu/cpu.h
> > > +++ b/include/qemu/cpu.h
> > > @@ -20,7 +20,7 @@
> > >  #ifndef QEMU_CPU_H
> > >  #define QEMU_CPU_H
> > >  
> > > -#include "qemu/object.h"
> > > +#include "hw/qdev-core.h"
> > >  #include "qemu-thread.h"
> > >  
> > >  /**
> > [...]
> > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > index 5b36046..d301f72 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -20,6 +20,7 @@
> > >  
> > >  #include "qemu/cpu.h"
> > >  #include "qemu-common.h"
> > > +#include "hw/qdev-core.h"
> > 
> > Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.
> > 
> > >  
> > >  void cpu_reset(CPUState *cpu)
> > >  {
> > > @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
> > >  
> > >  static void cpu_class_init(ObjectClass *klass, void *data)
> > >  {
> > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > >      CPUClass *k = CPU_CLASS(klass);
> > >  
> > >      k->reset = cpu_common_reset;
> > > +    dc->no_user = 1;
> > >  }
> > 
> > I wonder if we should add a comment that we are intentionally not
> > hooking up dc->reset (yet)?
> not relevant to this patch, could be separate patch though.
> 
> > 
> > >  
> > >  static TypeInfo cpu_type_info = {
> > 
> > Would like to add the missing const while touching this.
> > 
> > >      .name = TYPE_CPU,
> > > -    .parent = TYPE_OBJECT,
> > > +    .parent = TYPE_DEVICE,
> > >      .instance_size = sizeof(CPUState),
> > >      .abstract = true,
> > >      .class_size = sizeof(CPUClass),
> > 
> > My testing so far confirms that the combination of object_new() without
> > qdev_init[_nofail]() is working fine.
> +1, I tested this combo for (x86)-(softmmu|linux-user) targets, no issues were
> found so far.
> 
> > 
> > Using qdev_create() in the current state of stubs would lead to a silly
> > if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
> > expect qdev_create() to grow in functionality, so continuing to use
> > object_new() should be okay - SoCs like my Tegra model may want to use
> > object_initialize() so we cannot prescribe using qdev_create() anyway.
> > 
> > qdev_init_nofail() would call the qdev initfn (to be replaced by
> > realizefn, not used for CPU in this patch), then if no parent add it to
> > /machine/unassigned, register VMSD if not NULL, update the internal
> > state (blocking static property changes) and if hotplugged reset (unused
> > due to dc->no_user and lack of dc->reset). The /machine/unassigned part
> > may be interesting, e.g., for APIC modelling (so that we can model the
> > former ptr property / now pointer-setting as a link<> property).
> > 
> > With these considerations I am leaning towards accepting this patch if
> > nobody objects, so that we can move on to the next refactorings...
> +1
> 
> > 
> > Regards,
> > Andreas
> > 
>

Patch

diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
index 61b7698..bc004fd 100644
--- a/include/qemu/cpu.h
+++ b/include/qemu/cpu.h
@@ -20,7 +20,7 @@ 
 #ifndef QEMU_CPU_H
 #define QEMU_CPU_H
 
-#include "qemu/object.h"
+#include "hw/qdev-core.h"
 #include "qemu-thread.h"
 
 /**
@@ -46,7 +46,7 @@  typedef struct CPUState CPUState;
  */
 typedef struct CPUClass {
     /*< private >*/
-    ObjectClass parent_class;
+    DeviceClass parent_class;
     /*< public >*/
 
     void (*reset)(CPUState *cpu);
@@ -62,7 +62,7 @@  typedef struct CPUClass {
  */
 struct CPUState {
     /*< private >*/
-    Object parent_obj;
+    DeviceState parent_obj;
     /*< public >*/
 
     struct QemuThread *thread;
diff --git a/qom/cpu.c b/qom/cpu.c
index 5b36046..d301f72 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -20,6 +20,7 @@ 
 
 #include "qemu/cpu.h"
 #include "qemu-common.h"
+#include "hw/qdev-core.h"
 
 void cpu_reset(CPUState *cpu)
 {
@@ -36,14 +37,16 @@  static void cpu_common_reset(CPUState *cpu)
 
 static void cpu_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
     CPUClass *k = CPU_CLASS(klass);
 
     k->reset = cpu_common_reset;
+    dc->no_user = 1;
 }
 
 static TypeInfo cpu_type_info = {
     .name = TYPE_CPU,
-    .parent = TYPE_OBJECT,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(CPUState),
     .abstract = true,
     .class_size = sizeof(CPUClass),