| Message ID | 20191018150630.31099-8-damien.hedde@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On 10/18/19 5:06 PM, Damien Hedde wrote: > This commit make use of the resettable API to reset the device being > hotplugged during when it is realized. Also it make sure it is put in > a reset state coherent with the parent it is plugged into. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > > I'm not sure I've done everything that's required here since I do not > understand everything that's behind the hotplug and realize/unrealize. > I'm a bit lost there... > > One of the remaining question is: do we need to do things related to > unrealize ? > It seems, a device can be realized, unrealized, and re-realized ? But > is that true also for a hotplugged device ?> > Also resettable API is called there, so children if any are reset too. > This was not the case before, this a probably not a big deal, as long > as all children are realized too at this point. I'm not sure we have a > guarantee on this; the recursive realize is not done in the base bus > class so it will go only down to first buses level if it is not > propagated by bus subclasses. Do hotplug devices can have more than > single level bus subtree (ie: more than some child buses with no > devices on them) ? > --- > hw/core/qdev.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 3933f62d0c..c5d107ea4e 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -930,7 +930,14 @@ static void device_set_realized(Object *obj, bool value, Error **errp) > } > } > if (dev->hotplugged) { Just thinking that if it is possible to have un-realize then realize cycle, we probably need some code here (or even before realizing children) to ensure the reset state is zeroed before doing the following. > - device_legacy_reset(dev); > + /* > + * Reset the device, as well as its subtree which 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; > > -- Damien
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 3933f62d0c..c5d107ea4e 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -930,7 +930,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 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;
This commit make use of the resettable API to reset the device being hotplugged during when it is realized. Also it make sure it is put in a reset state coherent with the parent it is plugged into. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- I'm not sure I've done everything that's required here since I do not understand everything that's behind the hotplug and realize/unrealize. I'm a bit lost there... One of the remaining question is: do we need to do things related to unrealize ? It seems, a device can be realized, unrealized, and re-realized ? But is that true also for a hotplugged device ? Also resettable API is called there, so children if any are reset too. This was not the case before, this a probably not a big deal, as long as all children are realized too at this point. I'm not sure we have a guarantee on this; the recursive realize is not done in the base bus class so it will go only down to first buses level if it is not propagated by bus subclasses. Do hotplug devices can have more than single level bus subtree (ie: more than some child buses with no devices on them) ? --- hw/core/qdev.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)