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

[Peter Crosthwaite]

On Tue, Jul 7, 2015 at 7:22 AM, Christian Borntraeger
<address@hidden> 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
>
>