Re: [Qemu-devel] [PATCH for-2.4 1/2] core: reset handler for bus-less devices

[Cornelia Huck]

On Mon, 13 Jul 2015 17:05:31 +0200
Andreas Färber <address@hidden> wrote:

> Am 13.07.2015 um 16:30 schrieb Christian Borntraeger:
> > Am 13.07.2015 um 16:20 schrieb Andreas Färber:
> >> Am 13.07.2015 um 16:11 schrieb Cornelia Huck:
> >>> On Mon, 13 Jul 2015 14:22:05 +0200
> >>> Christian Borntraeger <address@hidden> wrote:
> >>>
> >>>> Am 09.07.2015 um 18:51 schrieb Cornelia Huck:
> >>>>> Devices that don't live on a bus aren't caught by the normal device
> >>>>> reset logic. Let's register a reset handler for those devices during
> >>>>> device realization that calls the reset handler for the associated
> >>>>> device class.
> >>>>>
> >>>>> Suggested-by: Peter Crosthwaite <address@hidden>
> >>>>> Signed-off-by: Cornelia Huck <address@hidden>
> >>>> reboot (from within guest) and external reset (system_reset in monitor)
> >>>> now work fine with the s390 watchdog.
> >>>>
> >>>> Tested-by: Christian Borntraeger <address@hidden>
> >>>>
> >>>>> ---
> >>>>>  hw/core/qdev.c | 15 +++++++++++++++
> >>>>>  1 file changed, 15 insertions(+)
> >>>
> >>> Thanks.
> >>>
> >>> Any objections against taking this through s390-next? I'd like to fix
> >>> diag288 reset (+ that annoying migration regession) for 2.4-rc1 and
> >>> send a pull request soon.
> >>
> >> Which device does this fix (only this diag88?), and is it really not
> >> possible to register a reset handler where it's being created?
> > 
> > A sysbus device reset is also not registered or called by its parent. It is 
> > resetted by the generic qdev handler walking all children (qdev_reset_all 
> > and 
> > qbus_reset_all), no?
> 
> It seems you're missing my point.
> 
> Having a SysBusDevice reset by the SysBus is the expected way, rather
> than having the SysBusDevice register its own handler. Reset propagates
> along defined paths - buses and composition - and doesn't hit randomly.
> Therefore resetting random bus-less devices is wrong by design. And a
> step backwards, too.
> 
> Which btw is the reason that my recursive realization patches stalled -
> we were potentially violating these rules.
> 
> Anyway, here it's not about a SysBusDevice, it's about a pure Device,
> unless I've misunderstood something.

So why does a pure Device have a reset callback then that is not called
by default?

Really, I think we're moving in circles here. First, the device should
not live on the sysbus as it does not fit the perceived sysbus
semantics. As there is no natural bus for it to live on, it becomes a
pure device. Which is not reset, because somehow a generic callback is
not called generically.

I think we're really doing ourselves a disservice by this confused
calling convention. How likely is it that someone introducing a pure
device is immediately aware that some callbacks happen automatically
while others don't? I wouldn't be surprised if there were some
headscratchers that are solved by adding the reset call.