[PATCH v5 07/13] hw/core/qdev: update hotplug reset regarding resettable

Damien Hedde posted 13 patches 6 years, 3 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, David Hildenbrand <david@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Fam Zheng <fam@euphon.net>, Matthew Rosato <mjrosato@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, John Snow <jsnow@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Richard Henderson <rth@twiddle.net>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>
There is a newer version of this series
[PATCH v5 07/13] hw/core/qdev: update hotplug reset regarding resettable
Posted by Damien Hedde 6 years, 3 months ago
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) {
-            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;
 
-- 
2.23.0


Re: [PATCH v5 07/13] hw/core/qdev: update hotplug reset regarding resettable
Posted by Damien Hedde 6 years, 3 months ago
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