Re: [Qemu-devel] [PATCH for-2.4] watchdog/diag288: correctly register for system reset requests

[Cornelia Huck <cornelia.huck@de.ibm.com>]

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.