From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: agraf@suse.de, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field
Date: Sun, 16 Dec 2012 19:04:18 +0200 [thread overview]
Message-ID: <20121216170418.GG15790@redhat.com> (raw)
In-Reply-To: <50C997BF.80200@redhat.com>
On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote:
> Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto:
> >>> Maybe it's obvious to you that qdev_reset_all(x)
> >>> does a soft reset and what soft reset means
> >>> for each bus type
> >>
> >> We can define soft reset to be *independent* of the bus type. As you
> >> said, you access it with a device register.
> >
> > I think qemu has one type of reset ATM which is the hard reset.
>
> A hard reset would kill BARs and configuration space too.
> qdev_reset_all doesn't. Ergo, it is not a hard reset.
>
> But hey, I'm not wed to names. Let's call it device-level reset and
> bus-level reset. Whatever.
It's not a question of a name.
ATM qemu supports one kind of reset because it's a kind of reset all
hardware supports. The moment we start inventing
our own one we need to document exactly what it means.
> > Any other kind is device specific now.
> > The fact that qdev has two APIs which are kind of but not
> > exactly the same is a bug not a feature.
>
> BusState reset and DeviceState reset are not the same because they are
> triggered differently, one by bus-level functionality (e.g. FLR) and the
> other by device-level functionality (e.g. a register).
>
> That's neither a bug nor a feature. It's just obvious.
>
> > And relying on it in generic virtio is just going to
> > confuse things.
>
> It is going to confuse you perhaps, but it will not confuse whoever will
> write the next virtio device with a bus. Who knows, virtio-i2c.
>
> > Further it does not follow that all backends are children
> > of the frontend.
>
> Backends are not children of the frontend. Seriously, this is qdev 101.
>
> virtio-scsi has no backend. Frontends (disks) are children of
> virtio-scsi. Each backend (host block device) is connected to a child
> of virtio-scsi.
>
> virtio-scsi does not need to reset back-ends. It needs to reset front-ends.
>
> virtio-serial does not know it needs to reset back-ends. It knows it
> needs to reset front-ends. Each reset of the front-end will also
> propagate to a back-end, but virtio-serial need not know that.
>
> > So please just fix the virtio-scsi bug for now and we can
> > address the bigger issue if any later.
>
> I'm refusing to fix the bug in virtio-scsi. It is not a virtio-scsi
> bug, as proved by the virtio-serial bus having to do the same things.
> Two wrongs do not make a right, and here we have three wrongs already:
> virtio-pci, virtio-s390, virtio-serial all reinventing the wheel.
>
> Paolo
You have a point about a problem.
My problem is with the solution, this solution depends on the exact
modeling for correctness and that I don't want to do since we seem to be
re-shuffling what inherits what so often.
For example I could not figure out how the reset function for virtio pci
(which clears pending msix vectors so is required) was called by this.
Another thing that bothers me is that during regular PCI bus reset
virtio does not invoke qdev_reset_all but with this reset, it does.
Inconsistent.
--
MST
next prev parent reply other threads:[~2012-12-16 17:14 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-12 14:26 [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field Paolo Bonzini
2012-12-12 14:26 ` [Qemu-devel] [PATCH 1/2] virtio-pci: " Paolo Bonzini
2012-12-12 14:44 ` Michael S. Tsirkin
2012-12-12 15:20 ` Paolo Bonzini
2012-12-12 15:33 ` Michael S. Tsirkin
2012-12-12 16:37 ` Paolo Bonzini
2012-12-12 17:05 ` Michael S. Tsirkin
2012-12-12 17:29 ` Paolo Bonzini
2012-12-12 21:32 ` Michael S. Tsirkin
2012-12-13 7:56 ` Paolo Bonzini
2012-12-12 14:26 ` [Qemu-devel] [PATCH 2/2] virtio-s390: " Paolo Bonzini
2012-12-12 15:38 ` [Qemu-devel] [PATCH 0/2] virtio: " Michael S. Tsirkin
2012-12-12 16:38 ` Paolo Bonzini
2012-12-12 17:18 ` Michael S. Tsirkin
[not found] ` <50C8BF83.4010307@redhat.com>
[not found] ` <20121212212720.GC23087@redhat.com>
2012-12-13 8:54 ` Paolo Bonzini
2012-12-16 17:04 ` Michael S. Tsirkin [this message]
2012-12-16 19:31 ` Paolo Bonzini
2012-12-16 21:15 ` Michael S. Tsirkin
2012-12-17 9:24 ` Paolo Bonzini
2012-12-17 10:40 ` Michael S. Tsirkin
2012-12-17 15:14 ` Paolo Bonzini
2012-12-17 15:24 ` Michael S. Tsirkin
2012-12-17 15:37 ` Paolo Bonzini
2012-12-17 16:01 ` Michael S. Tsirkin
2012-12-17 16:14 ` Paolo Bonzini
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=20121216170418.GG15790@redhat.com \
--to=mst@redhat.com \
--cc=agraf@suse.de \
--cc=pbonzini@redhat.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).