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

[Damien Hedde <damien.hedde@greensocs.com>]

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