| Message ID | 20200115123620.250132-8-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On 1/15/20 1:36 PM, Damien Hedde wrote: > This commit make use of the resettable API to reset the device being > hotplugged when it is realized. Also it ensures it is put in a reset > state coherent with the parent it is plugged into. > > Note that there is a difference in the reset. Instead of resetting > only the hotplugged device, we reset also its subtree (switch to > resettable API). This is not expected to be a problem because > sub-buses are just realized too. If a hotplugged device has any > sub-buses it is logical to reset them too at this point. > > The recently added should_be_hidden and PCI's partially_hotplugged > mechanisms do not interfere with realize operation: > + In the should_be_hidden use case, device creation is > delayed. > + The partially_hotplugged mechanism prevents a device to be > unplugged and unrealized from qdev POV and unrealized. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > --- > > v7 update: inline resettable_state_clear() > > v6 update: clear the reset state before doing any reset. Although it > is apparently highly improbable that a device be realized again after > being unrealized. See here for a discussion about this point and the > partially_hotplugged/shoud_be_hidden cases. > https://lists.nongnu.org/archive/html/qemu-devel/2019-11/msg04897.html > --- > include/hw/resettable.h | 11 +++++++++++ > hw/core/qdev.c | 15 ++++++++++++++- > 2 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/include/hw/resettable.h b/include/hw/resettable.h > index 3c87ab86c7..3f90da5b5b 100644 > --- a/include/hw/resettable.h > +++ b/include/hw/resettable.h > @@ -153,6 +153,17 @@ struct ResettableState { > bool exit_phase_in_progress; > }; > > +/** > + * resettable_state_clear: > + * Clear the state. It puts the state to the initial (zeroed) state required > + * to reuse an object. Typically used in realize step of base classes > + * implementing the interface. > + */ > +static inline void resettable_state_clear(ResettableState *state) > +{ > + memset(state, 0, sizeof(ResettableState)); > +} > + > /** > * resettable_reset: > * Trigger a reset on an object @obj of type @type. @obj must implement > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 310b87e25a..5fac4cd8fc 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -937,6 +937,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp) > } > } > > + /* > + * Clear the reset state, in case the object was previously unrealized > + * with a dirty state. > + */ > + resettable_state_clear(&dev->reset); > + > QLIST_FOREACH(bus, &dev->child_bus, sibling) { > object_property_set_bool(OBJECT(bus), true, "realized", > &local_err); > @@ -945,7 +951,14 @@ static void device_set_realized(Object *obj, bool value, Error **errp) > } > } > if (dev->hotplugged) { > - device_legacy_reset(dev); > + /* > + * Reset the device, as well as its subtree which, at this point, > + * should be realized too. > + */ > + resettable_assert_reset(OBJECT(dev), RESET_TYPE_COLD); > + resettable_change_parent(OBJECT(dev), OBJECT(dev->parent_bus), > + NULL); > + resettable_release_reset(OBJECT(dev), RESET_TYPE_COLD); > } > dev->pending_deleted_event = false; > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/include/hw/resettable.h b/include/hw/resettable.h index 3c87ab86c7..3f90da5b5b 100644 --- a/include/hw/resettable.h +++ b/include/hw/resettable.h @@ -153,6 +153,17 @@ struct ResettableState { bool exit_phase_in_progress; }; +/** + * resettable_state_clear: + * Clear the state. It puts the state to the initial (zeroed) state required + * to reuse an object. Typically used in realize step of base classes + * implementing the interface. + */ +static inline void resettable_state_clear(ResettableState *state) +{ + memset(state, 0, sizeof(ResettableState)); +} + /** * resettable_reset: * Trigger a reset on an object @obj of type @type. @obj must implement diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 310b87e25a..5fac4cd8fc 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -937,6 +937,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } } + /* + * Clear the reset state, in case the object was previously unrealized + * with a dirty state. + */ + resettable_state_clear(&dev->reset); + QLIST_FOREACH(bus, &dev->child_bus, sibling) { object_property_set_bool(OBJECT(bus), true, "realized", &local_err); @@ -945,7 +951,14 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } } if (dev->hotplugged) { - device_legacy_reset(dev); + /* + * Reset the device, as well as its subtree which, at this point, + * should be realized too. + */ + resettable_assert_reset(OBJECT(dev), RESET_TYPE_COLD); + resettable_change_parent(OBJECT(dev), OBJECT(dev->parent_bus), + NULL); + resettable_release_reset(OBJECT(dev), RESET_TYPE_COLD); } dev->pending_deleted_event = false;