Re: [PATCH v5 00/13] Multi-phase reset mechanism
On Fri, 18 Oct 2019 17:06:17 +0200 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. 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.
Re: [PATCH v5 00/13] Multi-phase reset mechanism
On Fri, 18 Oct 2019 at 16:07, 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. 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
Re: [PATCH v5 00/13] Multi-phase reset mechanism
On 11/8/19 4:28 PM, Peter Maydell wrote: > On Fri, 8 Nov 2019 at 15:26, Damien Hedde 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
Re: [PATCH v5 00/13] Multi-phase reset mechanism
On Fri, 8 Nov 2019 at 15:26, Damien Hedde 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
Re: [PATCH v5 00/13] Multi-phase reset mechanism
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
Re: [PATCH v5 00/13] Multi-phase reset mechanism
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 100
Re: [PATCH v5 00/13] Multi-phase reset mechanism
Patchew URL: https://patchew.org/QEMU/20191018150630.31099-1-damien.he...@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.1:/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 real1m44.422s user0m7.664s The full log is available at http://patchew.org/logs/20191018150630.31099-1-damien.he...@greensocs.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
