Re: [PATCH v5 00/13] Multi-phase reset mechanism

[Damien Hedde]

Hi,

Does anyone has comment about the interface / patch 3 ?
Should I try to split it ?

Thanks,
Damien

On 10/18/19 5:06 PM, Damien Hedde wrote:
> Hi all,
> 
> The purpose of this series is to split the current reset procedure
> into multiple phases. This will help to solve some ordering
> difficulties we have during reset. Previous version can be found here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg04359.html
> 
> This series adds resettable interface and transitions base Device and
> Bus classes (sysbus subclasses are ok too). It provides new reset
> functions but does not switch anymore the old functions
> (device_reset() and qdev/qbus_reset_all()) to resettable interface.
> These functions keep the exact same behavior as before.
> 
> The series also transition the main reset handlers registration which
> has no impact until devices and buses are transitioned.
> 
> I think this version is way better regarding the transition from the
> legacy to the resettable interface than the previous one.
> After this series, the plan is then to transition devices, buses and
> legacy reset call sites. Devices and buses have to be transitioned
> from mother class to daughter classes order but until the final
> (daughter) class is transitioned, old monolitic reset behavior will
> be kept for this class.
> 
> Changes v4 -> v5:
>   + various improvement in the resettable interface (regarding
>     transition, robustness and several reset types)
>   + better handling of transition from legacy reset to resettable
>   + device hotplug and parent bus 'hot' change support
>   + improved doc with examples and converted to rst format
> 
> Regarding the Resettable interface changes and how to handle more
> reset types, please read patch 3 message.
> 
> The series is organized as follows:
> Patch 1 is unmodified. Patch 2 adds some utility trace events.
> Patches 3 to 8 adds resettable api in devices and buses. Patch 9 adds
> some documentation. Patches 10 and 11 transition the call sites of
> qemu_register_reset(qdev/qbus_reset_all_fn, ...).
> 
> Apart from patch 7 about hotplug which is really a rfc. I think other
> patches are in pretty good shape.
> Patch 3 and 4 are quite big but I don't think it make much sense to
> split them. I could give it a try if you think it will ease reviews.
> Note that depending on what name we choose for device/bus reset
> functions (see patch 8), we may finally don't need patch 1.
> 
> I've also added patches 12 and 13 which handle the raspi sd card
> reparenting. I'm not sure they fit well in this series but at some
> point in this development I thought they had to be before patch 9
> (finally it is not the case). Since I had to develop some specific
> resettable support just for this case , I kept them as an example
> of what transition a device is. Note that patch 13 handle the only
> reset parent change (tricky) case I found (apart from hotplug).
> 
> Thanks for your feedback,
> Damien
> 
> Damien Hedde (13):
>   add device_legacy_reset function to prepare for reset api change
>   hw/core/qdev: add trace events to help with resettable transition
>   hw/core: create Resettable QOM interface
>   hw/core: add Resettable support to BusClass and DeviceClass
>   hw/core/resettable: add support for changing parent
>   hw/core/qdev: handle parent bus change regarding resettable
>   hw/core/qdev: update hotplug reset regarding resettable
>   hw/core: deprecate old reset functions and introduce new ones
>   docs/devel/reset.txt: add doc about Resettable interface
>   vl: replace deprecated qbus_reset_all registration
>   hw/s390x/ipl: replace deprecated qdev_reset_all registration
>   hw/gpio/bcm2835_gpio: Isolate sdbus reparenting
>   hw/gpio/bcm2835_gpio: Update to resettable
> 
>  Makefile.objs            |   1 +
>  docs/devel/reset.rst     | 282 ++++++++++++++++++++++++++++++++++++++
>  hw/audio/intel-hda.c     |   2 +-
>  hw/core/Makefile.objs    |   1 +
>  hw/core/bus.c            | 102 ++++++++++++++
>  hw/core/qdev.c           | 156 +++++++++++++++++++--
>  hw/core/resettable.c     | 289 +++++++++++++++++++++++++++++++++++++++
>  hw/core/trace-events     |  27 ++++
>  hw/gpio/bcm2835_gpio.c   |  31 +++--
>  hw/hyperv/hyperv.c       |   2 +-
>  hw/i386/pc.c             |   2 +-
>  hw/ide/microdrive.c      |   8 +-
>  hw/intc/spapr_xive.c     |   2 +-
>  hw/ppc/pnv_psi.c         |   2 +-
>  hw/ppc/spapr_pci.c       |   2 +-
>  hw/ppc/spapr_vio.c       |   2 +-
>  hw/s390x/ipl.c           |  10 +-
>  hw/s390x/s390-pci-inst.c |   2 +-
>  hw/scsi/vmw_pvscsi.c     |   2 +-
>  hw/sd/omap_mmc.c         |   2 +-
>  hw/sd/pl181.c            |   2 +-
>  include/hw/qdev-core.h   |  58 +++++++-
>  include/hw/resettable.h  | 224 ++++++++++++++++++++++++++++++
>  tests/Makefile.include   |   1 +
>  vl.c                     |  10 +-
>  25 files changed, 1185 insertions(+), 37 deletions(-)
>  create mode 100644 docs/devel/reset.rst
>  create mode 100644 hw/core/resettable.c
>  create mode 100644 include/hw/resettable.h
>