| Message ID | 20190523150543.22676-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] hw/core/bus.c: Only the main system bus can have no parent | expand |
On 5/23/19 5:05 PM, Peter Maydell wrote: > In commit 80376c3fc2c38fdd453 in 2010 we added a workaround for > some qbus buses not being connected to qdev devices -- if the > bus has no parent object then we register a reset function which > resets the bus on system reset (and unregister it when the > bus is unparented). > > Nearly a decade later, we have now no buses in the tree which > are created with non-NULL parents, so we can remove the > workaround and instead just assert that if the bus has a NULL > parent then it is the main system bus. > > (The absence of other parentless buses was confirmed by > code inspection of all the callsites of qbus_create() and > qbus_create_inplace() and cross-checked by 'make check'.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > --- > v1->v2: clean up also the bus_unparent() code > --- > hw/core/bus.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/hw/core/bus.c b/hw/core/bus.c > index e09843f6abe..b8839c7268d 100644 > --- a/hw/core/bus.c > +++ b/hw/core/bus.c > @@ -96,10 +96,9 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) > bus->parent->num_child_bus++; > object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL); > object_unref(OBJECT(bus)); > - } else if (bus != sysbus_get_default()) { > - /* TODO: once all bus devices are qdevified, > - only reset handler for main_system_bus should be registered here. */ > - qemu_register_reset(qbus_reset_all_fn, bus); > + } else { > + /* The only bus without a parent is the main system bus */ > + assert(bus == sysbus_get_default()); > } > } > > @@ -108,18 +107,16 @@ static void bus_unparent(Object *obj) > BusState *bus = BUS(obj); > BusChild *kid; > > + /* Only the main system bus has no parent, and that bus is never freed */ > + assert(bus->parent); > + > while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) { > DeviceState *dev = kid->child; > object_unparent(OBJECT(dev)); > } > - if (bus->parent) { > - QLIST_REMOVE(bus, sibling); > - bus->parent->num_child_bus--; > - bus->parent = NULL; > - } else { > - assert(bus != sysbus_get_default()); /* main_system_bus is never freed */ > - qemu_unregister_reset(qbus_reset_all_fn, bus); > - } > + QLIST_REMOVE(bus, sibling); > + bus->parent->num_child_bus--; > + bus->parent = NULL; > } > > void qbus_create_inplace(void *bus, size_t size, const char *typename, > Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
On 5/23/19 5:05 PM, Peter Maydell wrote: > In commit 80376c3fc2c38fdd453 in 2010 we added a workaround for > some qbus buses not being connected to qdev devices -- if the > bus has no parent object then we register a reset function which > resets the bus on system reset (and unregister it when the > bus is unparented). > > Nearly a decade later, we have now no buses in the tree which > are created with non-NULL parents, so we can remove the > workaround and instead just assert that if the bus has a NULL > parent then it is the main system bus. > > (The absence of other parentless buses was confirmed by > code inspection of all the callsites of qbus_create() and > qbus_create_inplace() and cross-checked by 'make check'.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > --- > v1->v2: clean up also the bus_unparent() code > --- > hw/core/bus.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/hw/core/bus.c b/hw/core/bus.c > index e09843f6abe..b8839c7268d 100644 > --- a/hw/core/bus.c > +++ b/hw/core/bus.c > @@ -96,10 +96,9 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) > bus->parent->num_child_bus++; > object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL); > object_unref(OBJECT(bus)); > - } else if (bus != sysbus_get_default()) { > - /* TODO: once all bus devices are qdevified, > - only reset handler for main_system_bus should be registered here. */ > - qemu_register_reset(qbus_reset_all_fn, bus); > + } else { > + /* The only bus without a parent is the main system bus */ > + assert(bus == sysbus_get_default()); > } > } > > @@ -108,18 +107,16 @@ static void bus_unparent(Object *obj) > BusState *bus = BUS(obj); > BusChild *kid; > > + /* Only the main system bus has no parent, and that bus is never freed */ > + assert(bus->parent); > + > while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) { > DeviceState *dev = kid->child; > object_unparent(OBJECT(dev)); > } > - if (bus->parent) { > - QLIST_REMOVE(bus, sibling); > - bus->parent->num_child_bus--; > - bus->parent = NULL; > - } else { > - assert(bus != sysbus_get_default()); /* main_system_bus is never freed */ > - qemu_unregister_reset(qbus_reset_all_fn, bus); > - } > + QLIST_REMOVE(bus, sibling); > + bus->parent->num_child_bus--; > + bus->parent = NULL; > } > > void qbus_create_inplace(void *bus, size_t size, const char *typename, > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/hw/core/bus.c b/hw/core/bus.c index e09843f6abe..b8839c7268d 100644 --- a/hw/core/bus.c +++ b/hw/core/bus.c @@ -96,10 +96,9 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) bus->parent->num_child_bus++; object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL); object_unref(OBJECT(bus)); - } else if (bus != sysbus_get_default()) { - /* TODO: once all bus devices are qdevified, - only reset handler for main_system_bus should be registered here. */ - qemu_register_reset(qbus_reset_all_fn, bus); + } else { + /* The only bus without a parent is the main system bus */ + assert(bus == sysbus_get_default()); } } @@ -108,18 +107,16 @@ static void bus_unparent(Object *obj) BusState *bus = BUS(obj); BusChild *kid; + /* Only the main system bus has no parent, and that bus is never freed */ + assert(bus->parent); + while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) { DeviceState *dev = kid->child; object_unparent(OBJECT(dev)); } - if (bus->parent) { - QLIST_REMOVE(bus, sibling); - bus->parent->num_child_bus--; - bus->parent = NULL; - } else { - assert(bus != sysbus_get_default()); /* main_system_bus is never freed */ - qemu_unregister_reset(qbus_reset_all_fn, bus); - } + QLIST_REMOVE(bus, sibling); + bus->parent->num_child_bus--; + bus->parent = NULL; } void qbus_create_inplace(void *bus, size_t size, const char *typename,