| Message ID | 20170214015843.GA2169@umbus.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/14/2017 02:58 AM, David Gibson wrote: > On Mon, Feb 13, 2017 at 10:33:24AM +0100, Cédric Le Goater wrote: >> XICSState inherits from SysBusDevice and so the object class should >> inherit from SysBusDeviceClass. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > That's definitely a bug, but I don't think this is the right fix. > > XICS was put on sysbus back when I thought that was where you put any > "system" devices that didn't belong on a more specific bus. I've > since discovered that's not the case, and - particularly since xics > doesn't expose any mmio - it shouldn't be a sysbus device at all. > > I've instead made a patch changing it to a direct child of > TYPE_DEVICE. Since the device shouldn't ever be command line > instantiated, and it has no migratable state I believe this should be > a safe change. > > This is in ppc-for-2.9, tentatively; please review: > > From eae21fcf2430588f17e6a58d505aa8eaa580b507 Mon Sep 17 00:00:00 2001 > From: David Gibson <david@gibson.dropbear.id.au> > Date: Tue, 14 Feb 2017 12:58:05 +1100 > Subject: [PATCH] xics: XICS should not be a SysBusDevice > > Currently xics - the component of the IBM POWER interrupt controller > representing the overall interrupt fabric / architecture is represented as > a descedent of SysBusDevice. However, this is not really correct - the descendant ? > xics presents nothing in MMIO space so it should be an "unattached" device > in the current QOM model. > > Since this device will always be created by the machine type, not created > specifically from the command line, and because it has no migrated state > it should be safe to move it around the device composition tree. > > Therefore this patch changes it to a descendent of TYPE_DEVICE, and makes > it an unattached device. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/intc/xics.c | 2 +- > hw/ppc/spapr.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 095c16a..372b831 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -260,7 +260,7 @@ static void xics_common_class_init(ObjectClass *oc, void *data) > > static const TypeInfo xics_common_info = { > .name = TYPE_XICS_COMMON, > - .parent = TYPE_SYS_BUS_DEVICE, > + .parent = TYPE_DEVICE, > .instance_size = sizeof(XICSState), > .class_size = sizeof(XICSStateClass), > .instance_init = xics_common_initfn, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 6f37288..9b6ad0f 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -101,7 +101,7 @@ static XICSState *try_create_xics(const char *type, int nr_servers, > Error *err = NULL; > DeviceState *dev; > > - dev = qdev_create(NULL, type); > + dev = DEVICE(object_new(type)); > qdev_prop_set_uint32(dev, "nr_servers", nr_servers); > qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); > object_property_set_bool(OBJECT(dev), true, "realized", &err); >
On Tue, Feb 14, 2017 at 07:25:07AM +0100, Cédric Le Goater wrote: > On 02/14/2017 02:58 AM, David Gibson wrote: > > On Mon, Feb 13, 2017 at 10:33:24AM +0100, Cédric Le Goater wrote: > >> XICSState inherits from SysBusDevice and so the object class should > >> inherit from SysBusDeviceClass. > >> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > That's definitely a bug, but I don't think this is the right fix. > > > > XICS was put on sysbus back when I thought that was where you put any > > "system" devices that didn't belong on a more specific bus. I've > > since discovered that's not the case, and - particularly since xics > > doesn't expose any mmio - it shouldn't be a sysbus device at all. > > > > I've instead made a patch changing it to a direct child of > > TYPE_DEVICE. Since the device shouldn't ever be command line > > instantiated, and it has no migratable state I believe this should be > > a safe change. > > > > This is in ppc-for-2.9, tentatively; please review: > > > > From eae21fcf2430588f17e6a58d505aa8eaa580b507 Mon Sep 17 00:00:00 2001 > > From: David Gibson <david@gibson.dropbear.id.au> > > Date: Tue, 14 Feb 2017 12:58:05 +1100 > > Subject: [PATCH] xics: XICS should not be a SysBusDevice > > > > Currently xics - the component of the IBM POWER interrupt controller > > representing the overall interrupt fabric / architecture is represented as > > a descedent of SysBusDevice. However, this is not really correct - the > > descendant ? descendent, actually, we were both wrong. > > > xics presents nothing in MMIO space so it should be an "unattached" device > > in the current QOM model. > > > > Since this device will always be created by the machine type, not created > > specifically from the command line, and because it has no migrated state > > it should be safe to move it around the device composition tree. > > > > Therefore this patch changes it to a descendent of TYPE_DEVICE, and makes > > it an unattached device. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks, > > C. > > > --- > > hw/intc/xics.c | 2 +- > > hw/ppc/spapr.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > index 095c16a..372b831 100644 > > --- a/hw/intc/xics.c > > +++ b/hw/intc/xics.c > > @@ -260,7 +260,7 @@ static void xics_common_class_init(ObjectClass *oc, void *data) > > > > static const TypeInfo xics_common_info = { > > .name = TYPE_XICS_COMMON, > > - .parent = TYPE_SYS_BUS_DEVICE, > > + .parent = TYPE_DEVICE, > > .instance_size = sizeof(XICSState), > > .class_size = sizeof(XICSStateClass), > > .instance_init = xics_common_initfn, > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 6f37288..9b6ad0f 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -101,7 +101,7 @@ static XICSState *try_create_xics(const char *type, int nr_servers, > > Error *err = NULL; > > DeviceState *dev; > > > > - dev = qdev_create(NULL, type); > > + dev = DEVICE(object_new(type)); > > qdev_prop_set_uint32(dev, "nr_servers", nr_servers); > > qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); > > object_property_set_bool(OBJECT(dev), true, "realized", &err); > > >
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 095c16a..372b831 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -260,7 +260,7 @@ static void xics_common_class_init(ObjectClass *oc, void *data) static const TypeInfo xics_common_info = { .name = TYPE_XICS_COMMON, - .parent = TYPE_SYS_BUS_DEVICE, + .parent = TYPE_DEVICE, .instance_size = sizeof(XICSState), .class_size = sizeof(XICSStateClass), .instance_init = xics_common_initfn, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 6f37288..9b6ad0f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -101,7 +101,7 @@ static XICSState *try_create_xics(const char *type, int nr_servers, Error *err = NULL; DeviceState *dev; - dev = qdev_create(NULL, type); + dev = DEVICE(object_new(type)); qdev_prop_set_uint32(dev, "nr_servers", nr_servers); qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); object_property_set_bool(OBJECT(dev), true, "realized", &err);