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

-- 
2.23.0

On Fri, 18 Oct 2019 17:06:17 +0200
Damien Hedde <damien.hedde@greensocs.com> 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.

I have looked over this patchset a bit (with an eye to the s390 stuff).
Seems sane, although I currently don't have the resources to review
more in detail.

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
> 

On 10/29/19 4:53 PM, Damien Hedde wrote:
> Hi,
> 
> Does anyone has comment about the interface / patch 3 ?
> Should I try to split it ?

ping

> 
> Thanks,
> Damien
> 

Thanks,
Damien

On Fri, 8 Nov 2019 at 15:26, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
>
> On 10/29/19 4:53 PM, Damien Hedde wrote:
> > Hi,
> >
> > Does anyone has comment about the interface / patch 3 ?
> > Should I try to split it ?
>
> ping

Hi; this patchset is still in my to-review queue, but we've
just gone into softfreeze for 4.2 so I'm a bit short on time
to look at anything that's not for this release.

I do definitely want to get this patchset in early in the
5.0 release cycle though.

thanks
-- PMM

On 11/8/19 4:28 PM, Peter Maydell wrote:
> On Fri, 8 Nov 2019 at 15:26, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>>
>> On 10/29/19 4:53 PM, Damien Hedde wrote:
>>> Hi,
>>>
>>> Does anyone has comment about the interface / patch 3 ?
>>> Should I try to split it ?
>>
>> ping
> 
> Hi; this patchset is still in my to-review queue, but we've
> just gone into softfreeze for 4.2 so I'm a bit short on time
> to look at anything that's not for this release.
> 
> I do definitely want to get this patchset in early in the
> 5.0 release cycle though.

Hi,
I understand,
Then I'll maybe try to advance more on the multiple reset type handling
and a do v6.

Thanks,
Damien

Patchew URL: https://patchew.org/QEMU/20191018150630.31099-1-damien.hedde@greensocs.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      hw/virtio/trace.o
  CC      hw/watchdog/trace.o

Warning, treated as error:
/tmp/qemu-test/src/docs/devel/reset.rst:document isn't included in any toctree
  CC      hw/xen/trace.o
  CC      hw/gpio/trace.o
---
  CC      stubs/bdrv-next-monitor-owned.o
  CC      stubs/blk-commit-all.o
  CC      stubs/blockdev-close-all-bdrv-states.o
make: *** [Makefile:994: docs/devel/index.html] Error 2
make: *** Waiting for unfinished jobs....
  CC      stubs/clock-warp.o
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=d026138e19654e3c80d1650d68316bd0', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-if7vrvxd/src/docker-src.2019-10-19-04.59.52.11111:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=d026138e19654e3c80d1650d68316bd0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-if7vrvxd/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    1m44.422s
user    0m7.664s


The full log is available at
http://patchew.org/logs/20191018150630.31099-1-damien.hedde@greensocs.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On Fri, 18 Oct 2019 at 16:07, Damien Hedde <damien.hedde@greensocs.com> 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.

Hi; I finally got back to reviewing this patchset, and I've
left comments on various patches (minor stuff only). I didn't
bother to comment on a few patches that I was happy with and
somebody else had already reviewed.

thanks
-- PMM