| Message ID | 1354726153-30264-9-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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
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
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
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
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.
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
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 >
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 > > >
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 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(-)