| Message ID | 20240813165250.2717650-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | s390: Convert virtio-ccw, cpu to three-phase reset, and followup cleanup | expand |
On 8/14/24 02:52, Peter Maydell wrote: > Convert the virtio-ccw code to three-phase reset. This allows us to > remove a call to device_class_set_parent_reset(), replacing it with > the three-phase equivalent resettable_class_set_parent_phases(). > Removing all the device_class_set_parent_reset() uses will allow us > to remove some of the glue code that interworks between three-phase > and legacy reset. > > This is a simple conversion, with no behavioural changes. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > It looks a little odd that we do the this-class reset first > and then chain up to the parent's reset, but that's what the > existing code does, so I left it alone. > --- > hw/s390x/virtio-ccw.h | 2 +- > hw/s390x/virtio-ccw.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 13/08/2024 18.52, Peter Maydell wrote: > Convert the virtio-ccw code to three-phase reset. This allows us to > remove a call to device_class_set_parent_reset(), replacing it with > the three-phase equivalent resettable_class_set_parent_phases(). > Removing all the device_class_set_parent_reset() uses will allow us > to remove some of the glue code that interworks between three-phase > and legacy reset. > > This is a simple conversion, with no behavioural changes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > It looks a little odd that we do the this-class reset first > and then chain up to the parent's reset, but that's what the > existing code does, so I left it alone. > --- > hw/s390x/virtio-ccw.h | 2 +- > hw/s390x/virtio-ccw.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 6 deletions(-) Acked-by: Thomas Huth <thuth@redhat.com> > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > index fac186c8f64..c7a830a1944 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -57,7 +57,7 @@ struct VirtIOCCWDeviceClass { > CCWDeviceClass parent_class; > void (*realize)(VirtioCcwDevice *dev, Error **errp); > void (*unrealize)(VirtioCcwDevice *dev); > - void (*parent_reset)(DeviceState *dev); > + ResettablePhases parent_phases; > }; > > /* Performance improves when virtqueue kick processing is decoupled from the > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index b4676909dd6..96747318d2a 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -913,14 +913,15 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector) > } > } > > -static void virtio_ccw_reset(DeviceState *d) > +static void virtio_ccw_reset_hold(Object *obj, ResetType type) > { > - VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(obj); > VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_GET_CLASS(dev); > > virtio_ccw_reset_virtio(dev); > - if (vdc->parent_reset) { > - vdc->parent_reset(d); > + > + if (vdc->parent_phases.hold) { > + vdc->parent_phases.hold(obj, type); > } > } > > @@ -1233,11 +1234,13 @@ static void virtio_ccw_device_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > CCWDeviceClass *k = CCW_DEVICE_CLASS(dc); > VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_CLASS(klass); > + ResettableClass *rc = RESETTABLE_CLASS(klass); > > k->unplug = virtio_ccw_busdev_unplug; > dc->realize = virtio_ccw_busdev_realize; > dc->unrealize = virtio_ccw_busdev_unrealize; > - device_class_set_parent_reset(dc, virtio_ccw_reset, &vdc->parent_reset); > + resettable_class_set_parent_phases(rc, NULL, virtio_ccw_reset_hold, NULL, > + &vdc->parent_phases); > } > > static const TypeInfo virtio_ccw_device_info = {
On Tue, 2024-08-13 at 17:52 +0100, Peter Maydell wrote: > Convert the virtio-ccw code to three-phase reset. This allows us to > remove a call to device_class_set_parent_reset(), replacing it with > the three-phase equivalent resettable_class_set_parent_phases(). > Removing all the device_class_set_parent_reset() uses will allow us > to remove some of the glue code that interworks between three-phase > and legacy reset. > > This is a simple conversion, with no behavioural changes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > It looks a little odd that we do the this-class reset first > and then chain up to the parent's reset, but that's what the > existing code does, so I left it alone. > --- Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Hi Peter, On 13/8/24 18:52, Peter Maydell wrote: > Convert the virtio-ccw code to three-phase reset. This allows us to > remove a call to device_class_set_parent_reset(), replacing it with > the three-phase equivalent resettable_class_set_parent_phases(). > Removing all the device_class_set_parent_reset() uses will allow us > to remove some of the glue code that interworks between three-phase > and legacy reset. > > This is a simple conversion, with no behavioural changes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > It looks a little odd that we do the this-class reset first > and then chain up to the parent's reset, but that's what the > existing code does, so I left it alone. Do you plan to post a follow up patch inverting the call order? Otherwise, could you add a comment in the code so we don't forget about this odd case? Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/s390x/virtio-ccw.h | 2 +- > hw/s390x/virtio-ccw.c | 13 ++++++++----- > 2 files changed, 9 insertions(+), 6 deletions(-) > -static void virtio_ccw_reset(DeviceState *d) > +static void virtio_ccw_reset_hold(Object *obj, ResetType type) > { > - VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(obj); > VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_GET_CLASS(dev); > > virtio_ccw_reset_virtio(dev); > - if (vdc->parent_reset) { > - vdc->parent_reset(d); > + > + if (vdc->parent_phases.hold) { > + vdc->parent_phases.hold(obj, type); > } > }
On Mon, 26 Aug 2024 at 17:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Hi Peter, > > On 13/8/24 18:52, Peter Maydell wrote: > > Convert the virtio-ccw code to three-phase reset. This allows us to > > remove a call to device_class_set_parent_reset(), replacing it with > > the three-phase equivalent resettable_class_set_parent_phases(). > > Removing all the device_class_set_parent_reset() uses will allow us > > to remove some of the glue code that interworks between three-phase > > and legacy reset. > > > > This is a simple conversion, with no behavioural changes. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > It looks a little odd that we do the this-class reset first > > and then chain up to the parent's reset, but that's what the > > existing code does, so I left it alone. > > Do you plan to post a follow up patch inverting the > call order? Otherwise, could you add a comment in the > code so we don't forget about this odd case? I didn't plan to do either because I don't know whether the s390 code relies on this or not and I don't want to investigate either... If somebody on the s390 side is interested in tracking that down they're welcome to :-) -- PMM
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index fac186c8f64..c7a830a1944 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -57,7 +57,7 @@ struct VirtIOCCWDeviceClass { CCWDeviceClass parent_class; void (*realize)(VirtioCcwDevice *dev, Error **errp); void (*unrealize)(VirtioCcwDevice *dev); - void (*parent_reset)(DeviceState *dev); + ResettablePhases parent_phases; }; /* Performance improves when virtqueue kick processing is decoupled from the diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index b4676909dd6..96747318d2a 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -913,14 +913,15 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector) } } -static void virtio_ccw_reset(DeviceState *d) +static void virtio_ccw_reset_hold(Object *obj, ResetType type) { - VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(obj); VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_GET_CLASS(dev); virtio_ccw_reset_virtio(dev); - if (vdc->parent_reset) { - vdc->parent_reset(d); + + if (vdc->parent_phases.hold) { + vdc->parent_phases.hold(obj, type); } } @@ -1233,11 +1234,13 @@ static void virtio_ccw_device_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); CCWDeviceClass *k = CCW_DEVICE_CLASS(dc); VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_CLASS(klass); + ResettableClass *rc = RESETTABLE_CLASS(klass); k->unplug = virtio_ccw_busdev_unplug; dc->realize = virtio_ccw_busdev_realize; dc->unrealize = virtio_ccw_busdev_unrealize; - device_class_set_parent_reset(dc, virtio_ccw_reset, &vdc->parent_reset); + resettable_class_set_parent_phases(rc, NULL, virtio_ccw_reset_hold, NULL, + &vdc->parent_phases); } static const TypeInfo virtio_ccw_device_info = {
Convert the virtio-ccw code to three-phase reset. This allows us to remove a call to device_class_set_parent_reset(), replacing it with the three-phase equivalent resettable_class_set_parent_phases(). Removing all the device_class_set_parent_reset() uses will allow us to remove some of the glue code that interworks between three-phase and legacy reset. This is a simple conversion, with no behavioural changes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- It looks a little odd that we do the this-class reset first and then chain up to the parent's reset, but that's what the existing code does, so I left it alone. --- hw/s390x/virtio-ccw.h | 2 +- hw/s390x/virtio-ccw.c | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-)