From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NFuQs-0001Pe-05 for qemu-devel@nongnu.org; Wed, 02 Dec 2009 14:00:10 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NFuQn-0001LX-Uj for qemu-devel@nongnu.org; Wed, 02 Dec 2009 14:00:09 -0500 Received: from [199.232.76.173] (port=37131 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NFuQn-0001LM-MO for qemu-devel@nongnu.org; Wed, 02 Dec 2009 14:00:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36849) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NFuQn-0008Ti-28 for qemu-devel@nongnu.org; Wed, 02 Dec 2009 14:00:05 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB2J042p001322 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 2 Dec 2009 14:00:04 -0500 Date: Wed, 2 Dec 2009 20:57:23 +0200 From: "Michael S. Tsirkin" Message-ID: <20091202185722.GE3984@redhat.com> References: <67f3d61df137cfd59fbf5da6b0b999cdf8d28758.1259754427.git.quintela@redhat.com> <20091202144333.GB18519@redhat.com> <20091202182707.GC3956@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH 15/41] virtio: remove save/load_queue for virtio List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On Wed, Dec 02, 2009 at 07:50:33PM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" wrote: > > On Wed, Dec 02, 2009 at 07:22:11PM +0100, Juan Quintela wrote: > >> "Michael S. Tsirkin" wrote: > >> > On Wed, Dec 02, 2009 at 01:04:13PM +0100, Juan Quintela wrote: > >> >> diff --git a/hw/virtio.c b/hw/virtio.c > >> >> index c136005..b565bf9 100644 > >> >> --- a/hw/virtio.c > >> >> +++ b/hw/virtio.c > >> >> @@ -643,8 +643,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) > >> >> qemu_put_be32(f, vdev->vq[i].vring.num); > >> >> qemu_put_be64(f, vdev->vq[i].pa); > >> >> qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); > >> >> - if (vdev->binding->save_queue) > >> >> - vdev->binding->save_queue(vdev->binding_opaque, i, f); > >> >> + if (vdev->type == VIRTIO_PCI && > >> >> + virtio_pci_msix_present(vdev->binding_opaque)) { > >> >> + qemu_put_be16s(f, &vdev->vq[i].vector); > >> >> + } Hmm, I just noticed that this also assumes that vector # fits in 16 bit. correct for PCI, but might be better not assume this in virtio.c > >> >> } > >> >> } > >> >> > >> > > >> > I think this will break build on systems without PCI > >> > because virtio_pci.c is not linked in there. > >> > Correct? > >> > >> It compiles for syborg (it has pci). There are no other users. > >> > >> > Making generic virtio.c depend on virtio_pci.c looks > >> > wrong in any case. We have bindings to > >> > resolve exactly this dependency problem. > >> > >> There is no way that you throw "this" blob to vmstate and it will know > >> what to do there. if it is needed, we can create an empty > >> virtio_pci_msix_present() function for !CONFIG_PCI. > >> > >> Later, Juan. > > > > That's not the idea. virtio knows nothing about msix etc. > > this is patently false :) I will agree if you would have done > s/kwnows/shouldn't know/ :) > > int nvectors; > > this is a field of VirtIODevice. > and there is another one in virtio-pci. > (master)$ grep -c nvectors hw/virtio.c > 0 > (master)$ > vectors are the abstraction that we use. > And you can see know what I mean by incestuous relation between virtio > <-> virtio-pci. To make things more interesting, it becomes a threesome > with msix :( These are just callbacks, no reason to call them names :) > > This belongs > > in the binding. If you want to know the number of vectors, please put > > something like ->get_nvectors in the binding and call that to figure out > > whether virtio has multivector. > > We could do it whatever. The big problem here is that virtio devices > are (normally) virtio devices and pci devices. There is no way that > would fit it well with qemu. > > There are two options: > > - having virtio contain callbacks from virtio-pci. > - having virtio-pci contain a virtio device. Second one sounds sane. virtio provides functions, virtio-pci calls them. > One is bad and the other is worse :( > > Will take a look at creating that get_nvectors() helper. > > Later, Juan.