qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cornelia.huck@de.ibm.com>
To: "Andreas Färber" <afaerber@suse.de>
Cc: peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org,
	agraf@suse.de, Christian Borntraeger <borntraeger@de.ibm.com>,
	jfrei@linux.vnet.ibm.com, gesaint@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH for-2.4 1/2] core: reset handler for bus-less devices
Date: Mon, 13 Jul 2015 17:38:58 +0200	[thread overview]
Message-ID: <20150713173858.1930cd71.cornelia.huck@de.ibm.com> (raw)
In-Reply-To: <55A3D3BB.3010508@suse.de>

On Mon, 13 Jul 2015 17:05:31 +0200
Andreas Färber <afaerber@suse.de> 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 <borntraeger@de.ibm.com> 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 <peter.crosthwaite@xilinx.com>
> >>>>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >>>> reboot (from within guest) and external reset (system_reset in monitor)
> >>>> now work fine with the s390 watchdog.
> >>>>
> >>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>>>
> >>>>> ---
> >>>>>  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.

  reply	other threads:[~2015-07-13 15:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-09 16:51 [Qemu-devel] [PATCH for-2.4 0/2] reset for bus-less devices Cornelia Huck
2015-07-09 16:51 ` [Qemu-devel] [PATCH for-2.4 1/2] core: reset handler " Cornelia Huck
2015-07-09 16:59   ` Andreas Färber
2015-07-09 18:53     ` Peter Crosthwaite
2015-07-13 14:28       ` Paolo Bonzini
2015-07-13 12:22   ` Christian Borntraeger
2015-07-13 14:11     ` Cornelia Huck
2015-07-13 14:20       ` Andreas Färber
2015-07-13 14:30         ` Christian Borntraeger
2015-07-13 15:05           ` Andreas Färber
2015-07-13 15:38             ` Cornelia Huck [this message]
2015-07-13 15:49               ` Andreas Färber
2015-07-13 15:56               ` Peter Maydell
2015-07-13 14:37         ` Cornelia Huck
2015-07-13 16:06           ` Andreas Färber
2015-07-14  7:41             ` Cornelia Huck
2015-07-09 16:51 ` [Qemu-devel] [PATCH for-2.4 2/2] watchdog/diag288: handle subsystem resets correctly Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150713173858.1930cd71.cornelia.huck@de.ibm.com \
    --to=cornelia.huck@de.ibm.com \
    --cc=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=borntraeger@de.ibm.com \
    --cc=gesaint@linux.vnet.ibm.com \
    --cc=jfrei@linux.vnet.ibm.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).