On Wed, Jul 31, 2019 at 11:09:05AM +0200, Damien Hedde wrote: > > > On 7/31/19 7:56 AM, David Gibson wrote: > > On Mon, Jul 29, 2019 at 04:56:25PM +0200, Damien Hedde wrote: > >> This add Resettable interface implementation for both Bus and Device. > >> > >> *resetting* counter and *reset_is_cold* flag are added in DeviceState > >> and BusState. > >> > >> Compatibility with existing code base is ensured. > >> The legacy bus or device reset method is called in the new exit phase > >> and the other 2 phases are let empty. Using the exit phase guarantee that > >> legacy resets are called in the "post" order (ie: children then parent) > >> in hierarchical reset. That is the same order as legacy qdev_reset_all > >> or qbus_reset_all were using. > >> > >> New *device_reset* and *bus_reset* function are proposed with an > >> additional boolean argument telling whether the reset is cold or warm. > >> Helper functions *device_reset_[warm|cold]* and *bus_reset_[warm|cold]* > >> are defined also as helpers. > >> > >> Also add a [device|bus]_is_resetting and [device|bus]_is_reset_cold > >> functions telling respectively whether the object is currently under reset > >> and > >> if the current reset is cold or not. > >> > >> Signed-off-by: Damien Hedde <damien.he...@greensocs.com> > >> --- > >> hw/core/bus.c | 85 ++++++++++++++++++++++++++++++++++++++++++ > >> hw/core/qdev.c | 82 ++++++++++++++++++++++++++++++++++++++++ > >> include/hw/qdev-core.h | 84 ++++++++++++++++++++++++++++++++++++++--- > >> tests/Makefile.include | 1 + > >> 4 files changed, 247 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/core/bus.c b/hw/core/bus.c > >> index 17bc1edcde..08a97addb6 100644 > >> --- a/hw/core/bus.c > >> +++ b/hw/core/bus.c > >> @@ -22,6 +22,7 @@ > >> #include "qemu/module.h" > >> #include "hw/qdev.h" > >> #include "qapi/error.h" > >> +#include "hw/resettable.h" > >> > >> void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error > >> **errp) > >> { > >> @@ -68,6 +69,75 @@ int qbus_walk_children(BusState *bus, > >> return 0; > >> } > >> > >> +void bus_reset(BusState *bus, bool cold) > >> +{ > >> + resettable_reset(OBJECT(bus), cold); > >> +} > >> + > >> +bool bus_is_resetting(BusState *bus) > >> +{ > >> + return (bus->resetting != 0); > >> +} > >> + > >> +bool bus_is_reset_cold(BusState *bus) > >> +{ > >> + return bus->reset_is_cold; > >> +} > >> + > >> +static uint32_t bus_get_reset_count(Object *obj) > >> +{ > >> + BusState *bus = BUS(obj); > >> + return bus->resetting; > >> +} > >> + > >> +static uint32_t bus_increment_reset_count(Object *obj) > >> +{ > >> + BusState *bus = BUS(obj); > >> + return ++bus->resetting; > >> +} > >> + > >> +static uint32_t bus_decrement_reset_count(Object *obj) > >> +{ > >> + BusState *bus = BUS(obj); > >> + return --bus->resetting; > >> +} > >> + > >> +static bool bus_set_reset_cold(Object *obj, bool cold) > >> +{ > >> + BusState *bus = BUS(obj); > >> + bool old = bus->reset_is_cold; > >> + bus->reset_is_cold = cold; > >> + return old; > >> +} > >> + > >> +static bool bus_set_hold_needed(Object *obj, bool hold_needed) > >> +{ > >> + BusState *bus = BUS(obj); > >> + bool old = bus->reset_hold_needed; > >> + bus->reset_hold_needed = hold_needed; > >> + return old; > >> +} > >> + > >> +static void bus_foreach_reset_child(Object *obj, void (*func)(Object *)) > >> +{ > >> + BusState *bus = BUS(obj); > >> + BusChild *kid; > >> + > >> + QTAILQ_FOREACH(kid, &bus->children, sibling) { > >> + func(OBJECT(kid->child)); > >> + } > >> +} > > > > IIUC, every resettable class would need more or less identical > > implementations of the above. That seems like an awful lot of > > boilerplate. > > Do you mean the get/increment_count/decrement_count, set_cold/hold part ? > True, but it's limited to the base classes. > Since Resettable is an interface, we have no state there to store what > we need. Only alternative is to have some kind of single > get_resettable_state method returning a pointer to the state (allowing > us to keep the functions in the interface code). > Beyond Device and Bus, which are done here, there is probably not so > many class candidates for the Resettable interface.
Right. I can't immediately see a better way to handle this, but it still seems like a mild code smell. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature