| Message ID | 20240119163512.3810301-6-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | buses: switch to 3-phase-reset | expand |
On Fri, Jan 19, 2024 at 04:35:12PM +0000, Peter Maydell wrote: > Date: Fri, 19 Jan 2024 16:35:12 +0000 > From: Peter Maydell <peter.maydell@linaro.org> > Subject: [PATCH 5/5] hw/core: Remove transitional infrastructure from > BusClass > X-Mailer: git-send-email 2.34.1 > > BusClass currently has transitional infrastructure to support > subclasses which implement the legacy BusClass::reset method rather > than the Resettable interface. We have now removed all the users of > BusClass::reset in the tree, so we can remove the transitional > infrastructure. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/hw/qdev-core.h | 2 -- > hw/core/bus.c | 67 ------------------------------------------ > 2 files changed, 69 deletions(-) Reviewed-by: Zhao Liu <zhao1.liu@intel.com> It seems the similar cleanup for DeviceClass needs a lot of effort. > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 151d9682380..986c924fa55 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -329,8 +329,6 @@ struct BusClass { > */ > char *(*get_fw_dev_path)(DeviceState *dev); > > - void (*reset)(BusState *bus); > - > /* > * Return whether the device can be added to @bus, > * based on the address that was set (via device properties) > diff --git a/hw/core/bus.c b/hw/core/bus.c > index c7831b5293b..b9d89495cdf 100644 > --- a/hw/core/bus.c > +++ b/hw/core/bus.c > @@ -232,57 +232,6 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev) > return g_strdup(object_get_typename(OBJECT(dev))); > } > > -/** > - * bus_phases_reset: > - * Transition reset method for buses to allow moving > - * smoothly from legacy reset method to multi-phases > - */ > -static void bus_phases_reset(BusState *bus) > -{ > - ResettableClass *rc = RESETTABLE_GET_CLASS(bus); > - > - if (rc->phases.enter) { > - rc->phases.enter(OBJECT(bus), RESET_TYPE_COLD); > - } > - if (rc->phases.hold) { > - rc->phases.hold(OBJECT(bus)); > - } > - if (rc->phases.exit) { > - rc->phases.exit(OBJECT(bus)); > - } > -} > - > -static void bus_transitional_reset(Object *obj) > -{ > - BusClass *bc = BUS_GET_CLASS(obj); > - > - /* > - * This will call either @bus_phases_reset (for multi-phases transitioned > - * buses) or a bus's specific method for not-yet transitioned buses. > - * In both case, it does not reset children. > - */ > - if (bc->reset) { > - bc->reset(BUS(obj)); > - } > -} > - > -/** > - * bus_get_transitional_reset: > - * check if the bus's class is ready for multi-phase > - */ > -static ResettableTrFunction bus_get_transitional_reset(Object *obj) > -{ > - BusClass *dc = BUS_GET_CLASS(obj); > - if (dc->reset != bus_phases_reset) { > - /* > - * dc->reset has been overridden by a subclass, > - * the bus is not ready for multi phase yet. > - */ > - return bus_transitional_reset; > - } > - return NULL; > -} > - > static void bus_class_init(ObjectClass *class, void *data) > { > BusClass *bc = BUS_CLASS(class); > @@ -293,22 +242,6 @@ static void bus_class_init(ObjectClass *class, void *data) > > rc->get_state = bus_get_reset_state; > rc->child_foreach = bus_reset_child_foreach; > - > - /* > - * @bus_phases_reset is put as the default reset method below, allowing > - * to do the multi-phase transition from base classes to leaf classes. It > - * allows a legacy-reset Bus class to extend a multi-phases-reset > - * Bus class for the following reason: > - * + If a base class B has been moved to multi-phase, then it does not > - * override this default reset method and may have defined phase methods. > - * + A child class C (extending class B) which uses > - * bus_class_set_parent_reset() (or similar means) to override the > - * reset method will still work as expected. @bus_phases_reset function > - * will be registered as the parent reset method and effectively call > - * parent reset phases. > - */ > - bc->reset = bus_phases_reset; > - rc->get_transitional_function = bus_get_transitional_reset; > } > > static void qbus_finalize(Object *obj) > -- > 2.34.1 > >
On Wed, 31 Jan 2024 at 03:44, Zhao Liu <zhao1.liu@intel.com> wrote: > > On Fri, Jan 19, 2024 at 04:35:12PM +0000, Peter Maydell wrote: > > Date: Fri, 19 Jan 2024 16:35:12 +0000 > > From: Peter Maydell <peter.maydell@linaro.org> > > Subject: [PATCH 5/5] hw/core: Remove transitional infrastructure from > > BusClass > > X-Mailer: git-send-email 2.34.1 > > > > BusClass currently has transitional infrastructure to support > > subclasses which implement the legacy BusClass::reset method rather > > than the Resettable interface. We have now removed all the users of > > BusClass::reset in the tree, so we can remove the transitional > > infrastructure. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > include/hw/qdev-core.h | 2 -- > > hw/core/bus.c | 67 ------------------------------------------ > > 2 files changed, 69 deletions(-) > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com> > > It seems the similar cleanup for DeviceClass needs a lot of effort. Yes and no. It's true that there are a lot of DeviceClass subclasses that implement the legacy reset method still. However, they are all "leaf" classes which provide a single reset method, and don't need to either chain up to a parent class reset implementation or allow a child class to inherit from them. So they don't affect any other classes in the system or block any other classes from being able to use a full three-phase reset. So I'm not too worried if we continue to have a lot of these leaf classes: the old reset method is then just a different way of having a device with a reset 'hold' method and no others. I did an earlier round of refactoring back in 2022 that did the cleanup of the non-leaf classes and the classes that need to chain up to their parent class reset, which mostly updated all of that kind of class to 3-phase. The one exception is that we still have a single user of device_class_set_parent_reset() in the s390 CPU, which is awkward to deal with because it interacts with an s390-specific handling of different levels of reset. I also want to look at providing a 3-phase equivalent of the qdev_register_reset() API at some point: that one is definitely blocking some useful improvements, including dealing with the vfio ordering issue, and also would let me clean up some M-profile ARM CPU reset hacks. thanks -- PMM
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 151d9682380..986c924fa55 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -329,8 +329,6 @@ struct BusClass { */ char *(*get_fw_dev_path)(DeviceState *dev); - void (*reset)(BusState *bus); - /* * Return whether the device can be added to @bus, * based on the address that was set (via device properties) diff --git a/hw/core/bus.c b/hw/core/bus.c index c7831b5293b..b9d89495cdf 100644 --- a/hw/core/bus.c +++ b/hw/core/bus.c @@ -232,57 +232,6 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev) return g_strdup(object_get_typename(OBJECT(dev))); } -/** - * bus_phases_reset: - * Transition reset method for buses to allow moving - * smoothly from legacy reset method to multi-phases - */ -static void bus_phases_reset(BusState *bus) -{ - ResettableClass *rc = RESETTABLE_GET_CLASS(bus); - - if (rc->phases.enter) { - rc->phases.enter(OBJECT(bus), RESET_TYPE_COLD); - } - if (rc->phases.hold) { - rc->phases.hold(OBJECT(bus)); - } - if (rc->phases.exit) { - rc->phases.exit(OBJECT(bus)); - } -} - -static void bus_transitional_reset(Object *obj) -{ - BusClass *bc = BUS_GET_CLASS(obj); - - /* - * This will call either @bus_phases_reset (for multi-phases transitioned - * buses) or a bus's specific method for not-yet transitioned buses. - * In both case, it does not reset children. - */ - if (bc->reset) { - bc->reset(BUS(obj)); - } -} - -/** - * bus_get_transitional_reset: - * check if the bus's class is ready for multi-phase - */ -static ResettableTrFunction bus_get_transitional_reset(Object *obj) -{ - BusClass *dc = BUS_GET_CLASS(obj); - if (dc->reset != bus_phases_reset) { - /* - * dc->reset has been overridden by a subclass, - * the bus is not ready for multi phase yet. - */ - return bus_transitional_reset; - } - return NULL; -} - static void bus_class_init(ObjectClass *class, void *data) { BusClass *bc = BUS_CLASS(class); @@ -293,22 +242,6 @@ static void bus_class_init(ObjectClass *class, void *data) rc->get_state = bus_get_reset_state; rc->child_foreach = bus_reset_child_foreach; - - /* - * @bus_phases_reset is put as the default reset method below, allowing - * to do the multi-phase transition from base classes to leaf classes. It - * allows a legacy-reset Bus class to extend a multi-phases-reset - * Bus class for the following reason: - * + If a base class B has been moved to multi-phase, then it does not - * override this default reset method and may have defined phase methods. - * + A child class C (extending class B) which uses - * bus_class_set_parent_reset() (or similar means) to override the - * reset method will still work as expected. @bus_phases_reset function - * will be registered as the parent reset method and effectively call - * parent reset phases. - */ - bc->reset = bus_phases_reset; - rc->get_transitional_function = bus_get_transitional_reset; } static void qbus_finalize(Object *obj)
BusClass currently has transitional infrastructure to support subclasses which implement the legacy BusClass::reset method rather than the Resettable interface. We have now removed all the users of BusClass::reset in the tree, so we can remove the transitional infrastructure. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/qdev-core.h | 2 -- hw/core/bus.c | 67 ------------------------------------------ 2 files changed, 69 deletions(-)