Re: [PATCH 06/10] hw/core: Add ResetContainer which holds objects implementing Resettable

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 21 Feb 2024 at 15:34, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 20/2/24 17:06, Peter Maydell wrote:
> > Implement a ResetContainer.  This is a subclass of Object, and it
> > implements the Resettable interface.  The container holds a list of
> > arbitrary other objects which implement Resettable, and when the
> > container is reset, all the objects it contains are also reset.
> >
> > This will allow us to have a 3-phase-reset equivalent of the old
> > qemu_register_reset() API: we will have a single "simulation reset"
> > top level ResetContainer, and objects in it are the equivalent of the
> > old QEMUResetHandler functions.
> >
> > The qemu_register_reset() API manages its list of callbacks using a
> > QTAILQ, but here we use a GPtrArray for our list of Resettable
> > children: we expect the "remove" operation (which will need to do an
> > iteration through the list) to be fairly uncommon, and we get simpler
> > code with fewer memory allocations.
> >
> > Since there is currently no listed owner in MAINTAINERS for the
> > existing reset-related source files, create a new section for
> > them, and add these new files there also.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   MAINTAINERS                      | 10 +++++
> >   include/hw/core/resetcontainer.h | 48 ++++++++++++++++++++
> >   hw/core/resetcontainer.c         | 76 ++++++++++++++++++++++++++++++++
> >   hw/core/meson.build              |  1 +
> >   4 files changed, 135 insertions(+)
> >   create mode 100644 include/hw/core/resetcontainer.h
> >   create mode 100644 hw/core/resetcontainer.c
>
>
> > +static void resettable_container_child_foreach(Object *obj,
> > +                                               ResettableChildCallback cb,
> > +                                               void *opaque, ResetType type)
> > +{
> > +    ResettableContainer *rc = RESETTABLE_CONTAINER(obj);
> > +    unsigned int len = rc->children->len;
> > +
> > +    for (unsigned int i = 0; i < len; i++) {
>
> Worth a pair of trace events around the callback call.

Do you think so? What would be the interest in them?
(The way the resettable handling works this foreach loop
gets called several times for any particular reset event,
as well as getting called if anybody calls qemu_unregister_reset():
so "something is iterating the resettable container children"
can be for multiple reasons.)

-- PMM