| Message ID | 1436259202-20509-2-git-send-email-cornelia.huck@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 07.07.2015 um 10:53 schrieb Cornelia Huck: > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > The diag288 watchdog is no sysbus device, therefore it doesn't get > triggered on resets automatically using dc->reset. > > Let's register the reset handler manually, so we get correctly notified > again when a system reset was requested. Also reset the watchdog on > subsystem resets that don't trigger a full system reset. > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> kdump/kexec and reboot disable the watchdog. Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 6 +++++- > hw/watchdog/wdt_diag288.c | 8 ++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 3d20d6a..4c51d1a 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { > > void io_subsystem_reset(void) > { > - DeviceState *css, *sclp, *flic; > + DeviceState *css, *sclp, *flic, *diag288; > > css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); > if (css) { > @@ -51,6 +51,10 @@ void io_subsystem_reset(void) > if (flic) { > qdev_reset_all(flic); > } > + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); > + if (diag288) { > + qdev_reset_all(diag288); > + } > } > > static int virtio_ccw_hcall_notify(const uint64_t *args) > diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c > index 1185e06..2a885a4 100644 > --- a/hw/watchdog/wdt_diag288.c > +++ b/hw/watchdog/wdt_diag288.c > @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) > timer_del(diag288->timer); > } > > +static void diag288_reset(void *opaque) > +{ > + DeviceState *diag288 = opaque; > + > + wdt_diag288_reset(diag288); > +} > + > static void diag288_timer_expired(void *dev) > { > qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); > @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) > { > DIAG288State *diag288 = DIAG288(dev); > > + qemu_register_reset(diag288_reset, diag288); > diag288->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, diag288_timer_expired, > dev); > } >
On Tue, Jul 7, 2015 at 1:53 AM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > The diag288 watchdog is no sysbus device, therefore it doesn't get > triggered on resets automatically using dc->reset. > > Let's register the reset handler manually, so we get correctly notified > again when a system reset was requested. Also reset the watchdog on > subsystem resets that don't trigger a full system reset. > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 6 +++++- > hw/watchdog/wdt_diag288.c | 8 ++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 3d20d6a..4c51d1a 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { > > void io_subsystem_reset(void) > { > - DeviceState *css, *sclp, *flic; > + DeviceState *css, *sclp, *flic, *diag288; > > css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); > if (css) { > @@ -51,6 +51,10 @@ void io_subsystem_reset(void) > if (flic) { > qdev_reset_all(flic); > } > + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); > + if (diag288) { > + qdev_reset_all(diag288); > + } > } > > static int virtio_ccw_hcall_notify(const uint64_t *args) > diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c > index 1185e06..2a885a4 100644 > --- a/hw/watchdog/wdt_diag288.c > +++ b/hw/watchdog/wdt_diag288.c > @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) > timer_del(diag288->timer); > } > > +static void diag288_reset(void *opaque) > +{ > + DeviceState *diag288 = opaque; > + > + wdt_diag288_reset(diag288); > +} > + > static void diag288_timer_expired(void *dev) > { > qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); > @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) > { > DIAG288State *diag288 = DIAG288(dev); > > + qemu_register_reset(diag288_reset, diag288); Doesn't seem right. Even if it is not a SBD it should still sit in the QOM tree in a place where the reset is reached. Where is this device in the QOM tree? I.E. What string do you get with an object_get_canonical_path() of the obj after machine init? Regards, Peter > diag288->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, diag288_timer_expired, > dev); > } > -- > 2.4.5 > >
Am 07.07.2015 um 12:51 schrieb Peter Crosthwaite: >> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); >> @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) >> { >> DIAG288State *diag288 = DIAG288(dev); >> >> + qemu_register_reset(diag288_reset, diag288); > > Doesn't seem right. Even if it is not a SBD it should still sit in the > QOM tree in a place where the reset is reached. Where is this device > in the QOM tree? Hmm, to me it seems that qemu_devices_reset does only work for devices on a bus. This is a busless-device so the reset function gets not triggered. > > I.E. What string do you get with an object_get_canonical_path() of the > obj after machine init? path /machine/peripheral/watchdog0
On Tue, 7 Jul 2015 03:51:59 -0700 Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Tue, Jul 7, 2015 at 1:53 AM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > > > The diag288 watchdog is no sysbus device, therefore it doesn't get > > triggered on resets automatically using dc->reset. > > > > Let's register the reset handler manually, so we get correctly notified > > again when a system reset was requested. Also reset the watchdog on > > subsystem resets that don't trigger a full system reset. > > > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > --- > > hw/s390x/s390-virtio-ccw.c | 6 +++++- > > hw/watchdog/wdt_diag288.c | 8 ++++++++ > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index 3d20d6a..4c51d1a 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { > > > > void io_subsystem_reset(void) > > { > > - DeviceState *css, *sclp, *flic; > > + DeviceState *css, *sclp, *flic, *diag288; > > > > css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); > > if (css) { > > @@ -51,6 +51,10 @@ void io_subsystem_reset(void) > > if (flic) { > > qdev_reset_all(flic); > > } > > + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); > > + if (diag288) { > > + qdev_reset_all(diag288); > > + } > > } > > > > static int virtio_ccw_hcall_notify(const uint64_t *args) > > diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c > > index 1185e06..2a885a4 100644 > > --- a/hw/watchdog/wdt_diag288.c > > +++ b/hw/watchdog/wdt_diag288.c > > @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) > > timer_del(diag288->timer); > > } > > > > +static void diag288_reset(void *opaque) > > +{ > > + DeviceState *diag288 = opaque; > > + > > + wdt_diag288_reset(diag288); > > +} > > + > > static void diag288_timer_expired(void *dev) > > { > > qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); > > @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) > > { > > DIAG288State *diag288 = DIAG288(dev); > > > > + qemu_register_reset(diag288_reset, diag288); > > Doesn't seem right. Even if it is not a SBD it should still sit in the > QOM tree in a place where the reset is reached. Where is this device > in the QOM tree? It will show up as /machine/peripheral-anon/device[]. But is just showing up really enough? For the sysbus, qbus_reset_all_fn is registered as a reset handler, and that called our reset handler when diag288 was still a sysbus device in a previous patch version.
On Tue, Jul 7, 2015 at 7:22 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > Am 07.07.2015 um 12:51 schrieb Peter Crosthwaite: >>> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); >>> @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) >>> { >>> DIAG288State *diag288 = DIAG288(dev); >>> >>> + qemu_register_reset(diag288_reset, diag288); >> >> Doesn't seem right. Even if it is not a SBD it should still sit in the >> QOM tree in a place where the reset is reached. Where is this device >> in the QOM tree? > > Hmm, to me it seems that qemu_devices_reset does only work for devices > on a bus. This is a busless-device so the reset function gets not triggered. > Yes I see. I think it is a core code bug though and we want to avoid having to patch individual devs based on their system level connectivity. I'm looking at qbus_realize, and there, there is code to register a reset for orphaned busses. So we have precedent for lazily setting up a reset for an orphaned bus at realize time, just not for indiv. devs. We can do the same. I think this can be added to device_set_realized(). If a devices parent is not a bus, then register its reset individually to catch-all these. There are other devs that will qualify as well. I remember a similar issue for NAND (hw/block/nand.c) where we lost it from the qtree because we removed it from the sysbus. Looking at the code, I fear NAND may be susceptible to this same missing-reset bug. Regards, Peter > >> >> I.E. What string do you get with an object_get_canonical_path() of the >> obj after machine init? > > > path /machine/peripheral/watchdog0 > >
On Tue, 7 Jul 2015 14:11:18 -0700 Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Tue, Jul 7, 2015 at 7:22 AM, Christian Borntraeger > <borntraeger@de.ibm.com> wrote: > > Am 07.07.2015 um 12:51 schrieb Peter Crosthwaite: > >>> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); > >>> @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) > >>> { > >>> DIAG288State *diag288 = DIAG288(dev); > >>> > >>> + qemu_register_reset(diag288_reset, diag288); > >> > >> Doesn't seem right. Even if it is not a SBD it should still sit in the > >> QOM tree in a place where the reset is reached. Where is this device > >> in the QOM tree? > > > > Hmm, to me it seems that qemu_devices_reset does only work for devices > > on a bus. This is a busless-device so the reset function gets not triggered. > > > > Yes I see. I think it is a core code bug though and we want to avoid > having to patch individual devs based on their system level > connectivity. I'm looking at qbus_realize, and there, there is code to > register a reset for orphaned busses. So we have precedent for lazily > setting up a reset for an orphaned bus at realize time, just not for > indiv. devs. We can do the same. > > I think this can be added to device_set_realized(). If a devices > parent is not a bus, then register its reset individually to catch-all > these. Solving this in the core sounds good, but do you already have some kind of patch ready? :) As we're pretty late in the cycle, it might make sense to merge the existing fix for diag288 first, and switch to a generic solution later on. > There are other devs that will qualify as well. I remember a > similar issue for NAND (hw/block/nand.c) where we lost it from the > qtree because we removed it from the sysbus. Looking at the code, I > fear NAND may be susceptible to this same missing-reset bug.
On 07/07/2015 12:51, Peter Crosthwaite wrote: > On Tue, Jul 7, 2015 at 1:53 AM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: >> From: Xu Wang <gesaint@linux.vnet.ibm.com> >> >> The diag288 watchdog is no sysbus device, therefore it doesn't get >> triggered on resets automatically using dc->reset. >> >> Let's register the reset handler manually, so we get correctly notified >> again when a system reset was requested. Also reset the watchdog on >> subsystem resets that don't trigger a full system reset. >> >> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> >> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> >> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> >> --- >> hw/s390x/s390-virtio-ccw.c | 6 +++++- >> hw/watchdog/wdt_diag288.c | 8 ++++++++ >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index 3d20d6a..4c51d1a 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { >> >> void io_subsystem_reset(void) >> { >> - DeviceState *css, *sclp, *flic; >> + DeviceState *css, *sclp, *flic, *diag288; >> >> css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); >> if (css) { >> @@ -51,6 +51,10 @@ void io_subsystem_reset(void) >> if (flic) { >> qdev_reset_all(flic); >> } >> + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); >> + if (diag288) { >> + qdev_reset_all(diag288); >> + } >> } >> >> static int virtio_ccw_hcall_notify(const uint64_t *args) >> diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c >> index 1185e06..2a885a4 100644 >> --- a/hw/watchdog/wdt_diag288.c >> +++ b/hw/watchdog/wdt_diag288.c >> @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) >> timer_del(diag288->timer); >> } >> >> +static void diag288_reset(void *opaque) >> +{ >> + DeviceState *diag288 = opaque; >> + >> + wdt_diag288_reset(diag288); >> +} >> + >> static void diag288_timer_expired(void *dev) >> { >> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); >> @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) >> { >> DIAG288State *diag288 = DIAG288(dev); >> >> + qemu_register_reset(diag288_reset, diag288); > > Doesn't seem right. Even if it is not a SBD it should still sit in the > QOM tree in a place where the reset is reached. Where is this device > in the QOM tree? Reset doesn't follow the QOM tree. In fact this is the main reason why I dislike TYPE_DEVICE as a superclass... Paolo
Am 08.07.2015 um 09:54 schrieb Paolo Bonzini: > > > On 07/07/2015 12:51, Peter Crosthwaite wrote: >> On Tue, Jul 7, 2015 at 1:53 AM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: >>> From: Xu Wang <gesaint@linux.vnet.ibm.com> >>> >>> The diag288 watchdog is no sysbus device, therefore it doesn't get >>> triggered on resets automatically using dc->reset. >>> >>> Let's register the reset handler manually, so we get correctly notified >>> again when a system reset was requested. Also reset the watchdog on >>> subsystem resets that don't trigger a full system reset. >>> >>> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> >>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> >>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> >>> --- >>> hw/s390x/s390-virtio-ccw.c | 6 +++++- >>> hw/watchdog/wdt_diag288.c | 8 ++++++++ >>> 2 files changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index 3d20d6a..4c51d1a 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { >>> >>> void io_subsystem_reset(void) >>> { >>> - DeviceState *css, *sclp, *flic; >>> + DeviceState *css, *sclp, *flic, *diag288; >>> >>> css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); >>> if (css) { >>> @@ -51,6 +51,10 @@ void io_subsystem_reset(void) >>> if (flic) { >>> qdev_reset_all(flic); >>> } >>> + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); >>> + if (diag288) { >>> + qdev_reset_all(diag288); >>> + } >>> } >>> >>> static int virtio_ccw_hcall_notify(const uint64_t *args) >>> diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c >>> index 1185e06..2a885a4 100644 >>> --- a/hw/watchdog/wdt_diag288.c >>> +++ b/hw/watchdog/wdt_diag288.c >>> @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) >>> timer_del(diag288->timer); >>> } >>> >>> +static void diag288_reset(void *opaque) >>> +{ >>> + DeviceState *diag288 = opaque; >>> + >>> + wdt_diag288_reset(diag288); >>> +} >>> + >>> static void diag288_timer_expired(void *dev) >>> { >>> qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); >>> @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) >>> { >>> DIAG288State *diag288 = DIAG288(dev); >>> >>> + qemu_register_reset(diag288_reset, diag288); >> >> Doesn't seem right. Even if it is not a SBD it should still sit in the >> QOM tree in a place where the reset is reached. Where is this device >> in the QOM tree? > > Reset doesn't follow the QOM tree. > > In fact this is the main reason why I dislike TYPE_DEVICE as a superclass... Oh, thats why we were asked to use that instead of sysbus device? ;-)
On 08/07/2015 10:01, Christian Borntraeger wrote: > > > Doesn't seem right. Even if it is not a SBD it should still sit in the > > > QOM tree in a place where the reset is reached. Where is this device > > > in the QOM tree? > > > > Reset doesn't follow the QOM tree. > > > > In fact this is the main reason why I dislike TYPE_DEVICE as a superclass... > > Oh, thats why we were asked to use that instead of sysbus device? ;-) Did I? I've always liked sysbus... :) Paolo
Am 08.07.2015 um 10:44 schrieb Paolo Bonzini: > > > On 08/07/2015 10:01, Christian Borntraeger wrote: >>>> Doesn't seem right. Even if it is not a SBD it should still sit in the >>>> QOM tree in a place where the reset is reached. Where is this device >>>> in the QOM tree? >>> >>> Reset doesn't follow the QOM tree. >>> >>> In fact this is the main reason why I dislike TYPE_DEVICE as a superclass... >> >> Oh, thats why we were asked to use that instead of sysbus device? ;-) > > Did I? I've always liked sysbus... :) Not you. But its a pattern that I have seen 3 or 4 times: "Oh no, please do not use xxx any more, we have fancy new stuff that nobody else is using in the tree, but its the way to go..." But most of the time the hickups can be handled. :-) Christian
On 08/07/2015 10:48, Christian Borntraeger wrote: > > > In fact this is the main reason why I dislike TYPE_DEVICE as a superclass... > > > > > > Oh, thats why we were asked to use that instead of sysbus device? ;-) > > > > Did I? I've always liked sysbus... :) > > Not you. > But its a pattern that I have seen 3 or 4 times: "Oh no, please do not > use xxx any more, we have fancy new stuff that nobody else is using in the tree, > but its the way to go..." But most of the time the hickups can be handled. :-) I know, and it's a problem. :( Paolo
Am 07.07.2015 um 10:53 schrieb Cornelia Huck: > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > The diag288 watchdog is no sysbus device, therefore it doesn't get > triggered on resets automatically using dc->reset. > > Let's register the reset handler manually, so we get correctly notified > again when a system reset was requested. Also reset the watchdog on > subsystem resets that don't trigger a full system reset. > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> Reviewed-by: Andreas Färber <afaerber@suse.de> Thanks, Andreas
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 3d20d6a..4c51d1a 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -36,7 +36,7 @@ typedef struct S390CcwMachineState { void io_subsystem_reset(void) { - DeviceState *css, *sclp, *flic; + DeviceState *css, *sclp, *flic, *diag288; css = DEVICE(object_resolve_path_type("", "virtual-css-bridge", NULL)); if (css) { @@ -51,6 +51,10 @@ void io_subsystem_reset(void) if (flic) { qdev_reset_all(flic); } + diag288 = DEVICE(object_resolve_path_type("", "diag288", NULL)); + if (diag288) { + qdev_reset_all(diag288); + } } static int virtio_ccw_hcall_notify(const uint64_t *args) diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c index 1185e06..2a885a4 100644 --- a/hw/watchdog/wdt_diag288.c +++ b/hw/watchdog/wdt_diag288.c @@ -40,6 +40,13 @@ static void wdt_diag288_reset(DeviceState *dev) timer_del(diag288->timer); } +static void diag288_reset(void *opaque) +{ + DeviceState *diag288 = opaque; + + wdt_diag288_reset(diag288); +} + static void diag288_timer_expired(void *dev) { qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); @@ -80,6 +87,7 @@ static void wdt_diag288_realize(DeviceState *dev, Error **errp) { DIAG288State *diag288 = DIAG288(dev); + qemu_register_reset(diag288_reset, diag288); diag288->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, diag288_timer_expired, dev); }