Re: [PATCH] docs/devel: add doc about device life cycles

[Peter Maydell]

On Fri, 22 Apr 2022 at 15:29, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Document the 3 life cycles cases that can happen with devices.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

Firstly, sorry it's taken me two months to get to this patch.
The underlying reason for this is that I'm not myself 100%
certain about how the QOM/qdev device lifecycle works and what
things should go in what lifecycle methods, so I didn't really
feel very confident about reviewing it...

To start with, I think we should definitely have some documentation
for this, and I like the structure you have here with:
 (1) the various ways devices are created and deleted
 (2) what the corresponding lifecycles are in terms of which
     methods get called
 (3) the concrete consequences for what a device should and
     should not do in each method

I'll try to get into some more detailed review below.

> diff --git a/docs/devel/device.rst b/docs/devel/device.rst
> new file mode 100644
> index 0000000000..80e3016e80
> --- /dev/null
> +++ b/docs/devel/device.rst

I think we should name the file device-lifecycle.rst -- we're
(hopefully) going to accumulate a bunch of documentation on devices
generally and we don't want it all to end up in this one file.

> @@ -0,0 +1,111 @@
> +QEMU device life-cycle
> +======================
> +
> +This document details the specifics of devices.
> +
> +Devices can be created in two ways: either internally by code or through a
> +user interface:
> +
> ++ command line interface provides ``-device`` option
> ++ QAPI interface provides ``device_add`` command

I think this bulleted list should list all the ways that devices
get created (and destroyed), so:

 Devices can be created in several ways:
  + programmatically, by the C code that implements board and SoC models
  + on the command line, via the -device option
  + via the QMP and HMP device_add monitor commands
  + temporarily as part of the introspection of device objects
    when the user asks for help on a device type or about what
    properties it implements
 In some cases, devices will also be destroyed:
  + if a device is hot-unpluggable then after an 'unplug' it will
    be destroyed
  + the temporary objects created for introspection are destroyed
    after they have been examined

 To do this, devices must implement at least some of these methods
 which are present on all QOM objects:
  + instance_init
  + instance_post_init
  + unparent
  + instance_finalize
 and these which are specific to devices:
  + realize
  + unrealize

 These methods will be called in fixed sequences by QEMU core code
 as the device is created, used, and destroyed. These sequences form
 the lifecycle of a device object. Understanding the possible
 lifecycles helps in working out which methods you need to implement
 and what code belongs in what method.


> +
> +Error handling is most important for the user interfaces. Internal code is
> +generally designed so that errors do not happen and if some happen, the error
> +is probably fatal (and QEMU will exit or abort).
> +
> +Devices are a particular type of QEMU objects. In addition of the
> +``instance_init``, ``instance_post_init``, ``unparent`` and
> +``instance_finalize`` methods (common to all QOM objects), they have the
> +additional methods:
> +
> ++ ``realize``
> ++ ``unrealize``
> +
> +In the following we will ignore ``instance_post_init`` and consider is
> +associated with ``instance_init``.
> +
> +``realize`` is the only method that can fail. In that case it should
> +return an adequate error. Some devices does not do this and should
> +not be created by means of user interfaces.

I don't think we really need to say that some of our device implementations
are buggy code :-)

> +
> +Device succesfully realized
> +---------------------------
> +
> +The normal use case for device is the following:
> +
> +1. ``instance_init``

   N. The device is configured by setting its QOM properties.

> +2. ``realize`` with success
> +3. The device takes part in emulation
> +4. ``unrealize`` and ``unparent``
> +5. ``instance_finalize``
> +
> +``instance_init`` and ``realize`` are part of the device creation procedure, 
> whereas
> +``unrealize`` and ``instance_finalize`` are part of the device deletion 
> procedure.

We should describe here what the difference is.

> +
> +In case of an object created by code, ``realize`` has to be done explicitly
> +(eg: by calling ``qdev_realize(...)``). This is done automatically in case 
> of a
> +device created via a user interface.
> +
> +On the other hand ``unrealize`` is done automatically.
> +``unparent`` will take care of unrealizing the device then undoing any bus
> +relationships (children and parent).

This (realize is done by calling qdev_realize, unrealize happens via unparent)
is part of how you use a device, not how you implement one. We might want
to document that, but that should be a separate document. Let's keep this one
to how the system looks from the point of view of a device implementation.

> +Note that ``instance_finalize`` may not occur just after ``unrealize`` 
> because
> +other objects than the parent can hold references to a device. It may even 
> not
> +happen at all if a reference is never released.
> +
> +Device realize failure
> +----------------------
> +
> +This use case is most important when the device is created through a user
> +interface. The user might for example invalid properties and in that case

"set invalid properties", I guess.

> +realize will fail and the device should then be deleted.
> +
> +1. ``instance_init``
> +2. ``realize`` failure
> +3. ``unparent``
> +4. ``instance_finalize``
> +
> +Failure to create a device should not leave traces. The emulation state after
> +that should be as if the device has not be created. ``realize`` and
> +``instance_finalize`` must take care of this.
> +
> +Device help
> +-----------

Call this "Device introspection" I think.

> +
> +Last use case is only a user interface case. When requesting help about a 
> device
> +type, the following life cycle exists:
> +
> +1. ``instance_init``
> +2. Introspect device properties
> +3. ``unparent``
> +4. ``instance_finalize``
> +
> +This use case is simple but it means that ``instance_finalize`` cannot 
> assume that
> +``realize`` has been called.
> +
> +Implementation consequences
> +---------------------------
> +
> +A device developer should ensure the above use cases are
> +supported so that the device is *user-creatable*.

Do we want to document the current requirements (every device has to
support the 'device introspection' cycle, hot pluggable/unpluggable
devices have to support full creation-and-deletion, devices that are
only cold-plugged or created by board models must support creation but
may not care about deletion), or some hypothetical hoped-for future
where the baseline for all devices is that they support the full
create-and-delete?

> +
> +In particular, fail cases must checked in realize and reported using the 
> error
> +parameter. One should particularly take care of cleaning correctly whatever 
> has
> +been previously done if realize fails. Cleaning tasks (eg: memory freeing) 
> can
> +be done in ``realize`` or ``instance_finalize`` as they will be called in a 
> row by
> +the user interface.
> +
> +To this end ``realize`` must ensure than no additional reference to the 
> device is
> +dangling when it fails. Otherwise the device will never be finalized and 
> deleted.
> +
> +If a device has created some children, they should be deleted as well in the
> +cleaning process. If ``object_initialize_child()`` was used to create a child
> +hosted into the device structure, the child memory space will disappear with 
> the
> +parent. No reference to such child must be dangling to ensure the child will
> +not survive its parent deletion.
> +
> +Although it is not asserted by code, one can assume ``realize`` will not be 
> tried
> +again in case of failure and that the device will be finalized if no 
> references
> +have been added during ``realize``.

I'm not sure what exactly this paragraph is trying to say. If our lifecycle
design says "realize only happens once" we can just document that that's
what the design is. We don't need to say whether or not something will assert().

I think there is scope for extending this last 'consequences' section to
be the place where we clearly say "Do X in instance_init; do Y in realize"
(possibly with annotations about whether nay particular case of that is
necessary or just convention).

-- PMM