On Thu, Jul 9, 2015 at 5:41 AM, Cornelia Huck <cornelia.h...@de.ibm.com> wrote:
> On Wed, 8 Jul 2015 12:31:40 +0200
> Cornelia Huck <cornelia.h...@de.ibm.com> wrote:
>
>> On Wed, 8 Jul 2015 09:45:05 +0200
>> Cornelia Huck <cornelia.h...@de.ibm.com> wrote:
>>
>> > On Tue, 7 Jul 2015 14:11:18 -0700
>> > Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
>>
>> > > 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.
>>
>> OTOH, this is less code than I expected. With the following code, I see
>> the diag288 reset callback called on system reset. If this looks good,
>> I can resend as a proper patch; we can reduce Xu's patch to the
>> io_subsystem_reset() part in that case. Opinions?
>
I'm for sending that new core patch, as I'm suspicious I can make it
fail in other places than your diag case. Runtime reset is poorly
exercised code so I think you are going to pickup half-a-dozen
bugfixes here.
Regards,
Peter
> Ping? Does this make sense?
>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index b2f404a..5c7c27b 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -1018,6 +1018,13 @@ static bool device_get_realized(Object *obj, Error
>> **errp)
>> return dev->realized;
>> }
>>
>> +static void do_device_reset(void *opaque)
>> +{
>> + DeviceState *dev = opaque;
>> +
>> + device_reset(dev);
>> +}
>> +
>> static void device_set_realized(Object *obj, bool value, Error **errp)
>> {
>> DeviceState *dev = DEVICE(obj);
>> @@ -1061,6 +1068,11 @@ static void device_set_realized(Object *obj, bool
>> value, Error **errp)
>> goto post_realize_fail;
>> }
>>
>> + if (!dev->parent_bus) {
>> + /* Make sure that reset is called for bus-less devices. */
>> + qemu_register_reset(do_device_reset, dev);
>> + }
>> +
>> if (qdev_get_vmsd(dev)) {
>> vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
>> dev->instance_id_alias,
>> @@ -1094,6 +1106,9 @@ static void device_set_realized(Object *obj, bool
>> value, Error **errp)
>> }
>> dev->pending_deleted_event = true;
>> DEVICE_LISTENER_CALL(unrealize, Reverse, dev);
>> + if (!dev->parent_bus) {
>> + qemu_unregister_reset(do_device_reset, dev);
>> + }
>> }
>>
>> if (local_err != NULL) {
>
>
